Skip to content

Commit 0451ea0

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 407e491 commit 0451ea0

2 files changed

Lines changed: 102 additions & 9 deletions

File tree

cmd/heygen/video_download.go

Lines changed: 63 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,11 @@ 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+
ExitCode: clierrors.ExitGeneral,
111+
}
108112
}
109113

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

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

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

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

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

205244
_, copyErr := io.Copy(tmp, resp.Body)
206245
closeErr := tmp.Close()
207246
if copyErr != nil {
208247
_ = os.Remove(tmpPath)
209-
return clierrors.New(fmt.Sprintf("download interrupted: %v", copyErr))
248+
return &clierrors.CLIError{
249+
Code: "download_interrupted",
250+
Message: fmt.Sprintf("download interrupted: %v", copyErr),
251+
Hint: "The transfer was cut off. Retry; the partial file was cleaned up.",
252+
ExitCode: clierrors.ExitGeneral,
253+
}
210254
}
211255
if closeErr != nil {
212256
_ = os.Remove(tmpPath)
213-
return clierrors.New(fmt.Sprintf("failed to finalize download: %v", closeErr))
257+
return &clierrors.CLIError{
258+
Code: "file_io_error",
259+
Message: fmt.Sprintf("failed to finalize download: %v", closeErr),
260+
Hint: "Could not finalize the file locally. Check free disk space.",
261+
ExitCode: clierrors.ExitGeneral,
262+
}
214263
}
215264

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

224278
return nil

cmd/heygen/video_download_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,11 +405,50 @@ func TestVideoDownload_DownloadFails(t *testing.T) {
405405
if res.ExitCode != 1 {
406406
t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
407407
}
408+
if !strings.Contains(res.Stderr, `"code":"asset_download_failed"`) {
409+
t.Fatalf("stderr should carry asset_download_failed (asset-host 5xx):\n%s", res.Stderr)
410+
}
408411
if _, err := os.Stat(dest); !os.IsNotExist(err) {
409412
t.Fatalf("expected no output file, stat err = %v", err)
410413
}
411414
}
412415

416+
// A presigned asset URL that 403s (or 404s) is expired/revoked — a client-side
417+
// download_url_expired, distinct from the asset host failing (asset_download_failed).
418+
func TestVideoDownload_ExpiredURL(t *testing.T) {
419+
tmpDir := t.TempDir()
420+
dest := filepath.Join(tmpDir, "expired.mp4")
421+
422+
var srv *httptest.Server
423+
srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
424+
switch {
425+
case r.Method == http.MethodGet && r.URL.Path == "/v3/videos/vid_123":
426+
writeJSON(t, w, map[string]any{
427+
"data": map[string]any{
428+
"video_url": srv.URL + "/download/expired.mp4",
429+
"status": "completed",
430+
},
431+
})
432+
case r.Method == http.MethodGet && r.URL.Path == "/download/expired.mp4":
433+
w.WriteHeader(http.StatusForbidden)
434+
default:
435+
t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
436+
}
437+
}))
438+
defer srv.Close()
439+
440+
res := runCommand(t, srv.URL, "test-key", "video", "download", "vid_123", "--output-path", dest)
441+
if res.ExitCode != 1 {
442+
t.Fatalf("ExitCode = %d, want 1\nstderr: %s", res.ExitCode, res.Stderr)
443+
}
444+
if !strings.Contains(res.Stderr, `"code":"download_url_expired"`) {
445+
t.Fatalf("stderr should carry download_url_expired (403 on presigned URL):\n%s", res.Stderr)
446+
}
447+
if !strings.Contains(res.Stderr, "video get") {
448+
t.Fatalf("expired-URL hint should point to re-fetching via video get:\n%s", res.Stderr)
449+
}
450+
}
451+
413452
func TestVideoDownload_AuthRequired(t *testing.T) {
414453
srv := setupTestServer(t, map[string]testHandler{})
415454
defer srv.Close()

0 commit comments

Comments
 (0)