Skip to content

Commit 1fcdae3

Browse files
somanshreddyclaude
andcommitted
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>
1 parent 844c843 commit 1fcdae3

2 files changed

Lines changed: 155 additions & 9 deletions

File tree

cmd/heygen/video_download.go

Lines changed: 64 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,12 @@ func newVideoDownloadCmd(ctx *cmdContext) *cobra.Command {
104104
"path": dest,
105105
})
106106
if err != nil {
107-
return clierrors.New(fmt.Sprintf("failed to encode response: %v", err))
107+
return &clierrors.CLIError{
108+
Code: "internal_error",
109+
Message: fmt.Sprintf("failed to encode response: %v", err),
110+
Hint: "Please report this CLI bug with the command you ran.",
111+
ExitCode: clierrors.ExitGeneral,
112+
}
108113
}
109114

110115
return ctx.formatter.Data(data, "", nil)
@@ -122,7 +127,12 @@ func extractAssetURL(raw json.RawMessage, videoID string, info assetInfo) (strin
122127
Data map[string]json.RawMessage `json:"data"`
123128
}
124129
if err := json.Unmarshal(raw, &resp); err != nil {
125-
return "", clierrors.New("failed to parse video response")
130+
return "", &clierrors.CLIError{
131+
Code: "response_parse_error",
132+
Message: "failed to parse the video response",
133+
Hint: "The API response could not be parsed. Retry; if it persists, report it (possible CLI/API mismatch).",
134+
ExitCode: clierrors.ExitGeneral,
135+
}
126136
}
127137

128138
var status string
@@ -180,45 +190,90 @@ func assetHint(field string) string {
180190
func downloadFile(ctx context.Context, videoURL, dest string) error {
181191
req, err := http.NewRequestWithContext(ctx, http.MethodGet, videoURL, nil)
182192
if err != nil {
183-
return clierrors.New(fmt.Sprintf("failed to build download request: %v", err))
193+
return &clierrors.CLIError{
194+
Code: "response_parse_error",
195+
Message: fmt.Sprintf("the download URL returned by the API is unusable: %v", err),
196+
Hint: "Re-fetch a fresh URL: heygen video get <video_id>",
197+
ExitCode: clierrors.ExitGeneral,
198+
}
184199
}
185200

186201
resp, err := downloadClient.Do(req)
187202
if err != nil {
188-
return clierrors.New(fmt.Sprintf("failed to download video: %v", err))
203+
return &clierrors.CLIError{
204+
Code: "network_error",
205+
Message: fmt.Sprintf("failed to download video: %v", err),
206+
Hint: "This is usually a temporary network issue. Check your connection and retry.",
207+
ExitCode: clierrors.ExitGeneral,
208+
}
189209
}
190210
defer resp.Body.Close()
191211

192212
if resp.StatusCode != http.StatusOK {
193-
return clierrors.New(fmt.Sprintf("download failed with HTTP %d", resp.StatusCode))
213+
// A presigned asset URL that 403s/404s is expired or revoked (client can
214+
// re-fetch); any other status is the asset host (our storage/CDN) failing.
215+
if resp.StatusCode == http.StatusForbidden || resp.StatusCode == http.StatusNotFound {
216+
return &clierrors.CLIError{
217+
Code: "download_url_expired",
218+
Message: fmt.Sprintf("download link expired or unavailable (HTTP %d)", resp.StatusCode),
219+
Hint: "This download link has expired. Re-fetch a fresh URL: heygen video get <video_id>",
220+
ExitCode: clierrors.ExitGeneral,
221+
}
222+
}
223+
return &clierrors.CLIError{
224+
Code: "asset_download_failed",
225+
Message: fmt.Sprintf("download failed with HTTP %d", resp.StatusCode),
226+
Hint: "The asset host returned an error fetching the file. This is usually transient — retry shortly.",
227+
ExitCode: clierrors.ExitGeneral,
228+
}
194229
}
195230

196231
// Write to a temp file in the same directory, then rename on success.
197232
// This prevents destroying an existing file on partial download failure.
198233
dir := filepath.Dir(dest)
199234
tmp, err := os.CreateTemp(dir, ".heygen-download-*.tmp")
200235
if err != nil {
201-
return clierrors.New(fmt.Sprintf("failed to create temp file in %q: %v", dir, err))
236+
return &clierrors.CLIError{
237+
Code: "file_io_error",
238+
Message: fmt.Sprintf("failed to create temp file in %q: %v", dir, err),
239+
Hint: "Could not write locally. Check the destination path and free disk space.",
240+
ExitCode: clierrors.ExitGeneral,
241+
}
202242
}
203243
tmpPath := tmp.Name()
204244

205245
_, copyErr := io.Copy(tmp, resp.Body)
206246
closeErr := tmp.Close()
207247
if copyErr != nil {
208248
_ = os.Remove(tmpPath)
209-
return clierrors.New(fmt.Sprintf("download interrupted: %v", copyErr))
249+
return &clierrors.CLIError{
250+
Code: "download_interrupted",
251+
Message: fmt.Sprintf("download interrupted: %v", copyErr),
252+
Hint: "The transfer was cut off. Retry; the partial file was cleaned up.",
253+
ExitCode: clierrors.ExitGeneral,
254+
}
210255
}
211256
if closeErr != nil {
212257
_ = os.Remove(tmpPath)
213-
return clierrors.New(fmt.Sprintf("failed to finalize download: %v", closeErr))
258+
return &clierrors.CLIError{
259+
Code: "file_io_error",
260+
Message: fmt.Sprintf("failed to finalize download: %v", closeErr),
261+
Hint: "Could not finalize the file locally. Check free disk space.",
262+
ExitCode: clierrors.ExitGeneral,
263+
}
214264
}
215265

216266
// Atomic rename. On Windows this may fail if dest is open elsewhere;
217267
// os.Rename across filesystems also fails, but temp file is in the
218268
// same directory so this is safe.
219269
if err := os.Rename(tmpPath, dest); err != nil {
220270
_ = os.Remove(tmpPath)
221-
return clierrors.New(fmt.Sprintf("failed to move download to %q: %v", dest, err))
271+
return &clierrors.CLIError{
272+
Code: "file_io_error",
273+
Message: fmt.Sprintf("failed to move download to %q: %v", dest, err),
274+
Hint: "Could not write to the destination path. Check permissions and free disk space.",
275+
ExitCode: clierrors.ExitGeneral,
276+
}
222277
}
223278

224279
return nil

cmd/heygen/video_download_test.go

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package main
22

33
import (
44
"encoding/json"
5+
"fmt"
56
"net/http"
67
"net/http/httptest"
78
"os"
@@ -405,11 +406,101 @@ func TestVideoDownload_DownloadFails(t *testing.T) {
405406
if res.ExitCode != 1 {
406407
t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
407408
}
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)
411+
}
408412
if _, err := os.Stat(dest); !os.IsNotExist(err) {
409413
t.Fatalf("expected no output file, stat err = %v", err)
410414
}
411415
}
412416

417+
// 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).
419+
func TestVideoDownload_ExpiredURL(t *testing.T) {
420+
for _, status := range []int{http.StatusForbidden, http.StatusNotFound} {
421+
t.Run(fmt.Sprintf("HTTP_%d", status), func(t *testing.T) {
422+
tmpDir := t.TempDir()
423+
dest := filepath.Join(tmpDir, "expired.mp4")
424+
425+
var srv *httptest.Server
426+
srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
427+
switch {
428+
case r.Method == http.MethodGet && r.URL.Path == "/v3/videos/vid_123":
429+
writeJSON(t, w, map[string]any{
430+
"data": map[string]any{
431+
"video_url": srv.URL + "/download/expired.mp4",
432+
"status": "completed",
433+
},
434+
})
435+
case r.Method == http.MethodGet && r.URL.Path == "/download/expired.mp4":
436+
w.WriteHeader(status)
437+
default:
438+
t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
439+
}
440+
}))
441+
defer srv.Close()
442+
443+
res := runCommand(t, srv.URL, "test-key", "video", "download", "vid_123", "--output-path", dest)
444+
if res.ExitCode != 1 {
445+
t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
446+
}
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)
449+
}
450+
if !strings.Contains(res.Stderr, "video get") {
451+
t.Fatalf("expired-URL hint should point to re-fetching via video get:\n%s", res.Stderr)
452+
}
453+
})
454+
}
455+
}
456+
457+
// The video-get response failing to parse, or yielding an unusable download URL,
458+
// classifies as response_parse_error (not the opaque error code).
459+
func TestVideoDownload_ResponseParseError(t *testing.T) {
460+
t.Run("malformed video-get JSON", func(t *testing.T) {
461+
dest := filepath.Join(t.TempDir(), "x.mp4")
462+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
463+
if r.URL.Path == "/v3/videos/vid_123" {
464+
w.WriteHeader(http.StatusOK)
465+
_, _ = w.Write([]byte("{ this is not valid json"))
466+
return
467+
}
468+
t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
469+
}))
470+
defer srv.Close()
471+
472+
res := runCommand(t, srv.URL, "test-key", "video", "download", "vid_123", "--output-path", dest)
473+
if res.ExitCode != 1 {
474+
t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
475+
}
476+
if !strings.Contains(res.Stderr, `"code":"response_parse_error"`) {
477+
t.Fatalf("stderr should carry response_parse_error:\n%s", res.Stderr)
478+
}
479+
})
480+
481+
t.Run("unusable download URL", func(t *testing.T) {
482+
dest := filepath.Join(t.TempDir(), "x.mp4")
483+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
484+
if r.URL.Path == "/v3/videos/vid_123" {
485+
writeJSON(t, w, map[string]any{
486+
"data": map[string]any{"video_url": "http://%zz", "status": "completed"},
487+
})
488+
return
489+
}
490+
t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
491+
}))
492+
defer srv.Close()
493+
494+
res := runCommand(t, srv.URL, "test-key", "video", "download", "vid_123", "--output-path", dest)
495+
if res.ExitCode != 1 {
496+
t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
497+
}
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)
500+
}
501+
})
502+
}
503+
413504
func TestVideoDownload_AuthRequired(t *testing.T) {
414505
srv := setupTestServer(t, map[string]testHandler{})
415506
defer srv.Close()

0 commit comments

Comments
 (0)