Commands: Count file download failures & pluralize logs

This commit is contained in:
Michael Mayer 2026-05-06 07:50:27 +00:00
parent d90bf1f5d5
commit bd7066e0c3
10 changed files with 201 additions and 12 deletions

View file

@ -340,6 +340,10 @@ func downloadAction(ctx *cli.Context) error {
if dlErr != nil {
log.Errorf("download failed: %v", dlErr)
// even on error, any completed files returned will be imported
if len(files) == 0 {
failures++
continue
}
}
// Ensure container/metadata per remux policy for file method
@ -370,13 +374,13 @@ func downloadAction(ctx *cli.Context) error {
elapsed := time.Since(start)
if failures > 0 {
log.Warnf("completed with %d error(s) in %s", failures, elapsed)
log.Warnf("completed with %s in %s", formatCount(failures, "error", "errors"), elapsed)
} else {
log.Infof("completed in %s", elapsed)
}
if failures > 0 {
return fmt.Errorf("some downloads failed: %d", failures)
return fmt.Errorf("%s", formatFailedCount(failures, "download", "downloads"))
}
return nil

View file

@ -3,6 +3,7 @@
package commands
import (
"bytes"
"os"
"path/filepath"
"runtime"
@ -58,7 +59,7 @@ func createFakeYtDlp(t *testing.T) string {
b.WriteString("for a in \"$@\"; do if [[ \"$a\" == \"--version\" ]]; then echo '2025.09.23'; exit 0; fi; done\n")
b.WriteString("OUT_TPL=\"\"\n")
b.WriteString("i=0; while [[ $i -lt $# ]]; do i=$((i+1)); arg=\"${!i}\"; if [[ \"$arg\" == \"--dump-single-json\" ]]; then echo '{\"id\":\"abc\",\"title\":\"Test\",\"url\":\"http://example.com\",\"_type\":\"video\"}'; exit 0; fi; if [[ \"$arg\" == \"--output\" ]]; then i=$((i+1)); OUT_TPL=\"${!i}\"; fi; done\n")
b.WriteString("if [[ $* == *'--print '* ]]; then OUT=\"$OUT_TPL\"; OUT=${OUT//%(id)s/abc}; OUT=${OUT//%(ext)s/mp4}; mkdir -p \"$(dirname \"$OUT\")\"; CONTENT=\"${YTDLP_DUMMY_CONTENT:-dummy}\"; echo \"$CONTENT\" > \"$OUT\"; echo \"$OUT\"; exit 0; fi\n")
b.WriteString("if [[ $* == *'--print '* ]]; then if [[ \"${YTDLP_FAIL_FILE_DOWNLOAD:-}\" == \"1\" ]]; then echo 'simulated yt-dlp failure' 1>&2; exit 1; fi; OUT=\"$OUT_TPL\"; OUT=${OUT//%(id)s/abc}; OUT=${OUT//%(ext)s/mp4}; mkdir -p \"$(dirname \"$OUT\")\"; CONTENT=\"${YTDLP_DUMMY_CONTENT:-dummy}\"; echo \"$CONTENT\" > \"$OUT\"; echo \"$OUT\"; exit 0; fi\n")
if err := os.WriteFile(path, []byte(b.String()), 0o755); err != nil {
t.Fatalf("failed to write fake yt-dlp: %v", err)
}
@ -217,9 +218,63 @@ func TestDownloadImpl_FileMethod_Always_RemuxFails(t *testing.T) {
if err == nil {
t.Fatalf("expected failure when remux is required but ffmpeg is unavailable")
}
if err.Error() != "1 download failed" {
t.Fatalf("unexpected error: %v", err)
}
// Cleanup destination folder if anything was created
c := get.Config()
outDir := filepath.Join(c.OriginalsPath(), dest)
_ = os.RemoveAll(outDir)
}
func TestDownloadImpl_FileMethod_ErrorWithoutFilesCountsFailure(t *testing.T) {
t.Setenv("YTDLP_FORCE_SHELL", "1")
t.Setenv("YTDLP_FAIL_FILE_DOWNLOAD", "1")
dl.ResetVersionWarningForTest()
fake := createFakeYtDlp(t)
orig := dl.YtDlpBin
defer func() { dl.YtDlpBin = orig }()
dest := "dl-e2e-file-error"
if c := get.Config(); c != nil {
c.Options().FFmpegBin = "/bin/false"
s := c.Settings()
s.Index.Convert = false
}
conf := get.Config()
if conf == nil {
t.Fatalf("missing test config")
}
conf.RegisterDb()
dl.YtDlpBin = fake
var logOutput bytes.Buffer
log.SetOutput(&logOutput)
t.Cleanup(func() {
log.SetOutput(os.Stderr)
})
var err error
err = runDownload(conf, DownloadOpts{
Dest: dest,
Method: "file",
FileRemux: "skip",
}, []string{"https://example.com/video"})
if err == nil {
t.Fatalf("expected file method failure when yt-dlp returns no files")
}
if err.Error() != "1 download failed" {
t.Fatalf("unexpected error: %v", err)
}
if !strings.Contains(logOutput.String(), "completed with 1 error in") {
t.Fatalf("expected singularized summary log, got: %q", logOutput.String())
}
c := get.Config()
outDir := filepath.Join(c.OriginalsPath(), dest)
_ = os.RemoveAll(outDir)
}

View file

@ -236,6 +236,10 @@ func runDownload(conf *config.Config, opts DownloadOpts, inputURLs []string) err
})
if err != nil {
log.Errorf("download failed: %v", err)
if len(files) == 0 {
failures++
continue
}
}
if fileRemux != "skip" {
for _, fp := range files {
@ -261,8 +265,8 @@ func runDownload(conf *config.Config, opts DownloadOpts, inputURLs []string) err
elapsed := time.Since(start)
if failures > 0 {
log.Warnf("completed with %d error(s) in %s", failures, elapsed)
return fmt.Errorf("some downloads failed: %d", failures)
log.Warnf("completed with %s in %s", formatCount(failures, "error", "errors"), elapsed)
return fmt.Errorf("%s", formatFailedCount(failures, "download", "downloads"))
}
log.Infof("completed in %s", elapsed)
return nil

View file

@ -1,9 +1,13 @@
package commands
import (
"bytes"
"os"
"strings"
"testing"
"github.com/photoprism/photoprism/internal/photoprism/dl"
"github.com/photoprism/photoprism/internal/photoprism/get"
)
func TestMissingFormatsHint(t *testing.T) {
@ -22,6 +26,7 @@ func TestMissingFormatsHint(t *testing.T) {
func TestResolveDownloadMethodEnv(t *testing.T) {
t.Setenv(downloadMethodEnv, "FILE")
method, fromEnv, err := resolveDownloadMethod("")
if err != nil {
t.Fatalf("unexpected error: %v", err)
@ -36,6 +41,7 @@ func TestResolveDownloadMethodEnv(t *testing.T) {
func TestResolveDownloadMethodInvalidEnv(t *testing.T) {
t.Setenv(downloadMethodEnv, "weird")
if _, _, err := resolveDownloadMethod(""); err == nil {
t.Fatalf("expected error for invalid env method")
}
@ -43,6 +49,7 @@ func TestResolveDownloadMethodInvalidEnv(t *testing.T) {
func TestResolveDownloadMethodFlagTakesPriority(t *testing.T) {
t.Setenv(downloadMethodEnv, "file")
method, fromEnv, err := resolveDownloadMethod("pipe")
if err != nil {
t.Fatalf("unexpected error: %v", err)
@ -54,3 +61,35 @@ func TestResolveDownloadMethodFlagTakesPriority(t *testing.T) {
t.Fatalf("did not expect env to be used when flag provided")
}
}
func TestRunDownload_InvalidURLsReportPluralFailures(t *testing.T) {
conf := get.Config()
if conf == nil {
t.Fatalf("missing test config")
}
conf.RegisterDb()
var logOutput bytes.Buffer
log.SetOutput(&logOutput)
t.Cleanup(func() {
log.SetOutput(os.Stderr)
})
err := runDownload(conf, DownloadOpts{
Dest: "dl-invalid",
}, []string{
"not a url",
"ftp://example.com/video.mp4",
})
if err == nil {
t.Fatalf("expected invalid URLs to fail")
}
if err.Error() != "2 downloads failed" {
t.Fatalf("unexpected error: %v", err)
}
if !strings.Contains(logOutput.String(), "completed with 2 errors in") {
t.Fatalf("expected pluralized summary log, got: %q", logOutput.String())
}
}

View file

@ -0,0 +1,17 @@
package commands
import (
"fmt"
"github.com/dustin/go-humanize/english"
)
// formatCount returns a human-readable count phrase with the correct noun form.
func formatCount(count int, singular, plural string) string {
return english.Plural(count, singular, plural)
}
// formatFailedCount returns a human-readable failure phrase with the correct noun form.
func formatFailedCount(count int, singular, plural string) string {
return fmt.Sprintf("%s failed", formatCount(count, singular, plural))
}

View file

@ -0,0 +1,46 @@
package commands
import "testing"
func TestFormatCount(t *testing.T) {
tests := []struct {
name string
count int
singular string
plural string
want string
}{
{name: "Zero", count: 0, singular: "error", plural: "errors", want: "0 errors"},
{name: "One", count: 1, singular: "download", plural: "downloads", want: "1 download"},
{name: "Many", count: 2, singular: "file", plural: "files", want: "2 files"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := formatCount(tt.count, tt.singular, tt.plural); got != tt.want {
t.Fatalf("formatCount() = %q, want %q", got, tt.want)
}
})
}
}
func TestFormatFailedCount(t *testing.T) {
tests := []struct {
name string
count int
singular string
plural string
want string
}{
{name: "One", count: 1, singular: "download", plural: "downloads", want: "1 download failed"},
{name: "Many", count: 2, singular: "file", plural: "files", want: "2 files failed"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := formatFailedCount(tt.count, tt.singular, tt.plural); got != tt.want {
t.Fatalf("formatFailedCount() = %q, want %q", got, tt.want)
}
})
}
}

View file

@ -95,10 +95,15 @@ func videoRemuxAction(ctx *cli.Context) error {
processed++
}
log.Infof("remux: processed %d, skipped %d, failed %d", processed, skipped, failed)
log.Infof(
"remux: processed %s, skipped %s, failed %s",
formatCount(processed, "file", "files"),
formatCount(skipped, "file", "files"),
formatCount(failed, "file", "files"),
)
if failed > 0 {
return fmt.Errorf("remux: %d files failed", failed)
return fmt.Errorf("remux: %s", formatFailedCount(failed, "file", "files"))
}
return nil

View file

@ -103,10 +103,15 @@ func videoTranscodeAction(ctx *cli.Context) error {
processed++
}
log.Infof("transcode: processed %d, skipped %d, failed %d", processed, skipped, failed)
log.Infof(
"transcode: processed %s, skipped %s, failed %s",
formatCount(processed, "file", "files"),
formatCount(skipped, "file", "files"),
formatCount(failed, "file", "files"),
)
if failed > 0 {
return fmt.Errorf("transcode: %d files failed", failed)
return fmt.Errorf("transcode: %s", formatFailedCount(failed, "file", "files"))
}
return nil

View file

@ -104,10 +104,15 @@ func videoTrimAction(ctx *cli.Context) error {
processed++
}
log.Infof("trim: processed %d, skipped %d, failed %d", processed, skipped, failed)
log.Infof(
"trim: processed %s, skipped %s, failed %s",
formatCount(processed, "file", "files"),
formatCount(skipped, "file", "files"),
formatCount(failed, "file", "files"),
)
if failed > 0 {
return fmt.Errorf("trim: %d files failed", failed)
return fmt.Errorf("trim: %s", formatFailedCount(failed, "file", "files"))
}
return nil

View file

@ -5,6 +5,8 @@ import (
"fmt"
"strings"
"github.com/dustin/go-humanize/english"
"github.com/photoprism/photoprism/internal/ai/face"
"github.com/photoprism/photoprism/internal/entity"
"github.com/photoprism/photoprism/internal/entity/query"
@ -63,7 +65,14 @@ func (w *Faces) OptimizeFor(subjUID string) (result FacesOptimizeResult, err err
"subject %s, iteration %d, cluster %d, count %d, ids %s",
}, subject, i, j, len(merge), clusterIDs)
log.Debugf("faces: retained manual clusters after merge: kept %d candidate cluster(s) [%s] for subject %s (merge) itr %d cluster %d", len(merge), clusterIDs, subject, i, j)
log.Debugf(
"faces: retained manual clusters after merge: kept %s [%s] for subject %s (merge) itr %d cluster %d",
english.Plural(len(merge), "candidate cluster", "candidate clusters"),
clusterIDs,
subject,
i,
j,
)
} else {
log.Errorf("%s (merge) itr %d cluster %d count %d", mergeErr, i, j, len(merge))
}