diff --git a/config.example.toml b/config.example.toml index 98eb0e1..4993a7d 100644 --- a/config.example.toml +++ b/config.example.toml @@ -79,3 +79,10 @@ mode = "copy" bwlimit = "2M" # optional = true # skip without failure if the local source is absent # (useful for detachable devices like external drives) +# max_runtime = "2h" # cap per-invocation wall-clock time; Shuttle kills + # the tool and reports "timed out" (counts as a + # failure, exit 1) if exceeded. Omit to disable. + # Format: Go duration string (e.g. "30s", "5m", + # "2h", "1h30m"). Applies per invocation: rsync + # jobs with N sources and rclone jobs with N + # remotes each get N independent timeouts. diff --git a/internal/config/config.go b/internal/config/config.go index 59eea08..cc488ea 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -11,6 +11,7 @@ import ( "os" "path/filepath" "strings" + "time" toml "github.com/pelletier/go-toml/v2" ) @@ -85,6 +86,12 @@ type Job struct { // sources (paths containing ":") since those are not stat'd before // invocation. Optional bool `toml:"optional"` + // MaxRuntime caps the wall-clock duration of each rsync/rclone + // invocation. Parsed via time.ParseDuration (e.g. "30m", "2h"). Empty + // means no timeout. Zero and negative durations are rejected at + // validation time. Applies per invocation: a job with N rsync sources + // or N rclone remotes gets N independent timeouts. + MaxRuntime string `toml:"max_runtime"` // Rsync fields Sources []string `toml:"sources"` @@ -167,6 +174,19 @@ func (c *Config) ResolvedLogRetentionDays() int { return *c.Defaults.LogRetentionDays } +// MaxRuntimeDuration returns the parsed max_runtime value, or 0 when +// the field is empty (meaning "no timeout"). The LoadBytes/LoadFile +// path validates max_runtime at parse time, so callers on that path +// never see the error. Callers that construct Job literals directly +// (e.g. tests) must handle the error explicitly so typos surface +// immediately instead of silently becoming "no timeout". +func (j Job) MaxRuntimeDuration() (time.Duration, error) { + if j.MaxRuntime == "" { + return 0, nil + } + return time.ParseDuration(j.MaxRuntime) +} + // JobNames returns the names of all configured jobs in config order. func (c *Config) JobNames() []string { names := make([]string, len(c.Jobs)) @@ -302,7 +322,7 @@ func validateRsyncJob(job Job) error { if job.Destination == "" { return fmt.Errorf("job %q: empty destination", job.Name) } - return nil + return validateMaxRuntime(job) } func validateRcloneJob(job Job) error { @@ -326,5 +346,26 @@ func validateRcloneJob(job Job) error { } remoteSeen[r] = true } + return validateMaxRuntime(job) +} + +// validateMaxRuntime enforces that max_runtime, when set, parses to a +// strictly positive time.Duration. An empty string means "no timeout" +// and passes. Zero and negative values are rejected so users don't +// accidentally configure "0s" expecting it to mean "no timeout". +func validateMaxRuntime(job Job) error { + if job.MaxRuntime == "" { + return nil + } + d, err := time.ParseDuration(job.MaxRuntime) + if err != nil { + return fmt.Errorf("job %q: invalid max_runtime %q: %w", job.Name, job.MaxRuntime, err) + } + if d == 0 { + return fmt.Errorf("job %q: max_runtime is %q; omit the field to disable the timeout", job.Name, job.MaxRuntime) + } + if d < 0 { + return fmt.Errorf("job %q: max_runtime must be positive (got %q)", job.Name, job.MaxRuntime) + } return nil } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 07d5366..c2f27ce 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -5,6 +5,7 @@ import ( "path/filepath" "strings" "testing" + "time" "github.com/jkleinne/shuttle/internal/config" ) @@ -546,3 +547,155 @@ destination = "/tmp/backup" t.Error("Jobs[0].Optional = true, want false (zero value)") } } + +func TestLoad_MaxRuntime_ParsedAndExposed(t *testing.T) { + tomlData := ` +[[job]] +name = "photos" +engine = "rsync" +sources = ["/tmp/photos"] +destination = "/tmp/backup" +max_runtime = "2h" +` + cfg, err := config.LoadBytes([]byte(tomlData)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.Jobs[0].MaxRuntime != "2h" { + t.Errorf("MaxRuntime = %q, want \"2h\"", cfg.Jobs[0].MaxRuntime) + } + got, err := cfg.Jobs[0].MaxRuntimeDuration() + if err != nil { + t.Fatalf("MaxRuntimeDuration() error = %v", err) + } + if got != 2*time.Hour { + t.Errorf("MaxRuntimeDuration() = %v, want 2h", got) + } +} + +func TestLoad_MaxRuntime_Absent_DurationIsZero(t *testing.T) { + tomlData := ` +[[job]] +name = "photos" +engine = "rsync" +sources = ["/tmp/photos"] +destination = "/tmp/backup" +` + cfg, err := config.LoadBytes([]byte(tomlData)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.Jobs[0].MaxRuntime != "" { + t.Errorf("MaxRuntime = %q, want empty", cfg.Jobs[0].MaxRuntime) + } + got, err := cfg.Jobs[0].MaxRuntimeDuration() + if err != nil { + t.Fatalf("MaxRuntimeDuration() error = %v", err) + } + if got != 0 { + t.Errorf("MaxRuntimeDuration() = %v, want 0", got) + } +} + +func TestLoad_MaxRuntime_Zero_Rejected(t *testing.T) { + tomlData := ` +[[job]] +name = "photos" +engine = "rsync" +sources = ["/tmp/photos"] +destination = "/tmp/backup" +max_runtime = "0s" +` + _, err := config.LoadBytes([]byte(tomlData)) + if err == nil { + t.Fatal("expected error for max_runtime = \"0s\", got nil") + } + msg := err.Error() + if !strings.Contains(msg, "photos") || !strings.Contains(msg, "max_runtime") { + t.Errorf("error %q should mention job name and field", msg) + } +} + +func TestLoad_MaxRuntime_Negative_Rejected(t *testing.T) { + tomlData := ` +[[job]] +name = "photos" +engine = "rsync" +sources = ["/tmp/photos"] +destination = "/tmp/backup" +max_runtime = "-5m" +` + _, err := config.LoadBytes([]byte(tomlData)) + if err == nil { + t.Fatal("expected error for negative max_runtime, got nil") + } + if !strings.Contains(err.Error(), "photos") { + t.Errorf("error %q should mention job name", err.Error()) + } + if !strings.Contains(err.Error(), "positive") { + t.Errorf("error %q should mention \"positive\"", err.Error()) + } +} + +func TestLoad_MaxRuntime_Malformed_Rejected(t *testing.T) { + tomlData := ` +[[job]] +name = "photos" +engine = "rsync" +sources = ["/tmp/photos"] +destination = "/tmp/backup" +max_runtime = "banana" +` + _, err := config.LoadBytes([]byte(tomlData)) + if err == nil { + t.Fatal("expected error for malformed max_runtime, got nil") + } + if !strings.Contains(err.Error(), "photos") { + t.Errorf("error %q should mention job name", err.Error()) + } +} + +func TestLoad_MaxRuntime_RcloneJob_ValidatedAndExposed(t *testing.T) { + // validateMaxRuntime is called from both validateRsyncJob and + // validateRcloneJob. The other tests cover the rsync path; this + // pins that the validator runs for rclone jobs too. + tomlData := ` +[[job]] +name = "docs-to-cloud" +engine = "rclone" +source = "/tmp/docs" +remotes = ["my_gdrive"] +mode = "copy" +max_runtime = "30m" +` + cfg, err := config.LoadBytes([]byte(tomlData)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + got, err := cfg.Jobs[0].MaxRuntimeDuration() + if err != nil { + t.Fatalf("MaxRuntimeDuration() error = %v", err) + } + if got != 30*time.Minute { + t.Errorf("MaxRuntimeDuration() = %v, want 30m", got) + } +} + +func TestLoad_MaxRuntime_RcloneJob_NegativeRejected(t *testing.T) { + tomlData := ` +[[job]] +name = "docs-to-cloud" +engine = "rclone" +source = "/tmp/docs" +remotes = ["my_gdrive"] +mode = "copy" +max_runtime = "-1h" +` + _, err := config.LoadBytes([]byte(tomlData)) + if err == nil { + t.Fatal("expected error for negative max_runtime on rclone job, got nil") + } + if !strings.Contains(err.Error(), "docs-to-cloud") { + t.Errorf("error %q should mention job name", err.Error()) + } +} diff --git a/internal/engine/rclone.go b/internal/engine/rclone.go index 3655c25..b57a317 100644 --- a/internal/engine/rclone.go +++ b/internal/engine/rclone.go @@ -136,8 +136,13 @@ func (e *RcloneExecutor) Exec(ctx context.Context, args []string, onProgress fun } if err := cmd.Start(); err != nil { - e.logger.FileError(fmt.Sprintf("rclone start failed for %s: %v", displayName, err)) - return ItemResult{Name: displayName, Status: StatusFailed} + status := classifyExitStatus(ctx, err) + if status == StatusTimedOut { + e.logger.FileError(fmt.Sprintf("rclone timed out for %s after per-job max_runtime: %v", displayName, err)) + } else { + e.logger.FileError(fmt.Sprintf("rclone start failed for %s: %v", displayName, err)) + } + return ItemResult{Name: displayName, Status: status} } var pipeWg sync.WaitGroup @@ -168,14 +173,17 @@ func (e *RcloneExecutor) Exec(ctx context.Context, args []string, onProgress fun } stats.Elapsed = elapsed - status := StatusOK + status := classifyExitStatus(ctx, runErr) if runErr != nil { - status = StatusFailed subcommand := "rclone" if len(args) > 0 { subcommand = "rclone " + args[0] } - e.logger.FileError(fmt.Sprintf("%s failed for %s: %v", subcommand, displayName, runErr)) + if status == StatusTimedOut { + e.logger.FileError(fmt.Sprintf("%s timed out for %s after per-job max_runtime: %v", subcommand, displayName, runErr)) + } else { + e.logger.FileError(fmt.Sprintf("%s failed for %s: %v", subcommand, displayName, runErr)) + } } return ItemResult{Name: displayName, Status: status, Stats: stats} diff --git a/internal/engine/rclone_test.go b/internal/engine/rclone_test.go index 7e3682f..e9326ee 100644 --- a/internal/engine/rclone_test.go +++ b/internal/engine/rclone_test.go @@ -483,3 +483,29 @@ func TestCleanupArchives_KeepsRecent(t *testing.T) { t.Error("recent archive dir should still exist") } } + +func TestRcloneExec_ExpiredContext_ReturnsTimedOut(t *testing.T) { + skipIfNoRclone(t) + + src := t.TempDir() + dst := t.TempDir() + if err := os.WriteFile(filepath.Join(src, "hello.txt"), []byte("world"), 0o644); err != nil { + t.Fatalf("writing test file: %v", err) + } + + // A context whose deadline is already in the past will cause exec.CommandContext + // to kill the process immediately, producing a DeadlineExceeded context error. + ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-1*time.Second)) + defer cancel() + + executor, logPath := newRcloneTestExecutor(t) + // --config /dev/null prevents rclone from loading the developer's + // real rclone config during the test. + job := config.Job{ExtraFlags: []string{"--config", "/dev/null"}} + args := BuildRcloneArgs("copy", nil, job, src+"/", ":local:"+dst, false, logPath, "") + result := executor.Exec(ctx, args, nil) + + if result.Status != StatusTimedOut { + t.Errorf("Status = %q, want %q", result.Status, StatusTimedOut) + } +} diff --git a/internal/engine/render.go b/internal/engine/render.go index 7231a6a..f69e47d 100644 --- a/internal/engine/render.go +++ b/internal/engine/render.go @@ -36,6 +36,7 @@ const ( symbolOptionalMissing = "○" labelFailed = "failed" labelNotFound = "not found" + labelTimedOut = "timed out" labelSkipped = "skipped" labelOptionalMissing = "source missing (optional)" tallyLabelPassed = "passed" @@ -56,7 +57,7 @@ func colorize(useColor bool, code, text string) string { // statusSymbol returns a colored status indicator character. func statusSymbol(status Status, useColor bool) string { switch status { - case StatusFailed, StatusNotFound: + case StatusFailed, StatusNotFound, StatusTimedOut: return colorize(useColor, ansiRed, symbolFailed) case StatusSkipped: return colorize(useColor, ansiYellow, symbolSkipped) @@ -75,6 +76,8 @@ func itemStatsText(item ItemResult, useColor bool) string { return colorize(useColor, ansiRed, labelFailed) case StatusNotFound: return colorize(useColor, ansiRed, labelNotFound) + case StatusTimedOut: + return colorize(useColor, ansiRed, labelTimedOut) case StatusSkipped: return colorize(useColor, ansiYellow, labelSkipped) case StatusOptionalMissing: diff --git a/internal/engine/render_test.go b/internal/engine/render_test.go new file mode 100644 index 0000000..7779b65 --- /dev/null +++ b/internal/engine/render_test.go @@ -0,0 +1,65 @@ +package engine + +import ( + "strings" + "testing" +) + +func TestStatusSymbol_TimedOut_SharesFailedGlyph(t *testing.T) { + // StatusTimedOut is a failure; it must share the ✗ glyph with StatusFailed. + got := statusSymbol(StatusTimedOut, false) + if got != symbolFailed { + t.Errorf("statusSymbol(StatusTimedOut, false) = %q, want %q", got, symbolFailed) + } +} + +func TestStatusSymbol_TimedOut_ColorEnabled_WrapsRed(t *testing.T) { + got := statusSymbol(StatusTimedOut, true) + want := colorize(true, ansiRed, symbolFailed) + if got != want { + t.Errorf("statusSymbol(StatusTimedOut, true) = %q, want %q", got, want) + } +} + +func TestItemStatsText_TimedOut_PlainText(t *testing.T) { + item := ItemResult{Status: StatusTimedOut} + got := itemStatsText(item, false) + if got != labelTimedOut { + t.Errorf("itemStatsText(StatusTimedOut, false) = %q, want %q", got, labelTimedOut) + } +} + +func TestItemStatsText_TimedOut_ColorEnabled_WrapsRed(t *testing.T) { + item := ItemResult{Status: StatusTimedOut} + got := itemStatsText(item, true) + want := colorize(true, ansiRed, labelTimedOut) + if got != want { + t.Errorf("itemStatsText(StatusTimedOut, true) = %q, want %q", got, want) + } +} + +func TestRenderSummary_TimedOut_ContainsTimedOutAndTalliesAsFailed(t *testing.T) { + s := Summary{ + Jobs: []JobResult{ + { + Name: "backup", + Items: []ItemResult{ + {Name: "backup", Status: StatusTimedOut}, + }, + }, + }, + } + var buf strings.Builder + RenderSummary(&buf, s, false) + output := buf.String() + + if !strings.Contains(output, "timed out") { + t.Errorf("RenderSummary output does not contain %q:\n%s", "timed out", output) + } + if !strings.Contains(output, "✗ backup") { + t.Errorf("RenderSummary output does not contain %q (failure glyph on job line):\n%s", "✗ backup", output) + } + if !strings.Contains(output, "1 failed") { + t.Errorf("RenderSummary output does not contain %q:\n%s", "1 failed", output) + } +} diff --git a/internal/engine/rsync.go b/internal/engine/rsync.go index bc88e8f..15625ad 100644 --- a/internal/engine/rsync.go +++ b/internal/engine/rsync.go @@ -121,8 +121,13 @@ func (e *RsyncExecutor) Exec(ctx context.Context, args []string, onProgress func cmd.Stderr = &stderrBuf if err := cmd.Start(); err != nil { - e.logger.FileError(fmt.Sprintf("rsync start failed for %s: %v", source, err)) - return ItemResult{Name: name, Status: StatusFailed} + status := classifyExitStatus(ctx, err) + if status == StatusTimedOut { + e.logger.FileError(fmt.Sprintf("rsync timed out for %s after per-job max_runtime: %v", source, err)) + } else { + e.logger.FileError(fmt.Sprintf("rsync start failed for %s: %v", source, err)) + } + return ItemResult{Name: name, Status: status} } var pipeWg sync.WaitGroup @@ -148,10 +153,13 @@ func (e *RsyncExecutor) Exec(ctx context.Context, args []string, onProgress func stats := ParseRsyncStats(capture.Bytes()) stats.Elapsed = elapsed - status := StatusOK + status := classifyExitStatus(ctx, runErr) if runErr != nil { - status = StatusFailed - e.logger.FileError(fmt.Sprintf("rsync failed for %s: %v", source, runErr)) + if status == StatusTimedOut { + e.logger.FileError(fmt.Sprintf("rsync timed out for %s after per-job max_runtime: %v", source, runErr)) + } else { + e.logger.FileError(fmt.Sprintf("rsync failed for %s: %v", source, runErr)) + } } return ItemResult{Name: name, Status: status, Stats: stats} diff --git a/internal/engine/rsync_test.go b/internal/engine/rsync_test.go index cf38fb2..9abd1f2 100644 --- a/internal/engine/rsync_test.go +++ b/internal/engine/rsync_test.go @@ -4,9 +4,11 @@ import ( "bytes" "context" "os" + "os/exec" "path/filepath" "strings" "testing" + "time" "github.com/jkleinne/shuttle/internal/config" "github.com/jkleinne/shuttle/internal/log" @@ -206,3 +208,30 @@ func TestScanRsyncProgress_CallsOnProgress(t *testing.T) { t.Errorf("last progress = %q, want something containing 45%%", called[len(called)-1]) } } + +func TestRsyncExec_ExpiredContext_ReturnsTimedOut(t *testing.T) { + if _, err := exec.LookPath("rsync"); err != nil { + t.Skip("rsync not found on PATH") + } + + src := t.TempDir() + dst := t.TempDir() + if err := os.WriteFile(filepath.Join(src, "hello.txt"), []byte("world"), 0o644); err != nil { + t.Fatalf("writing test file: %v", err) + } + + // A context whose deadline is already in the past will cause exec.CommandContext + // to kill the process immediately, producing a DeadlineExceeded context error. + ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-1*time.Second)) + defer cancel() + + defaults := &config.RsyncDefaults{Flags: []string{"-a"}} + args := BuildRsyncArgs(defaults, config.Job{}, src+"/", dst+"/", false, false, "") + + executor := NewRsyncExecutor(newTestLogger(t)) + result := executor.Exec(ctx, args, nil) + + if result.Status != StatusTimedOut { + t.Errorf("Status = %q, want %q", result.Status, StatusTimedOut) + } +} diff --git a/internal/engine/runner.go b/internal/engine/runner.go index 7302cfc..99db851 100644 --- a/internal/engine/runner.go +++ b/internal/engine/runner.go @@ -3,6 +3,7 @@ package engine import ( "context" "crypto/sha256" + "errors" "fmt" "io" "os" @@ -183,6 +184,9 @@ func (r *Runner) dispatchRclone(ctx context.Context, job config.Job, opts RunOpt results := make([]JobResult, 0, len(remotes)) for _, remote := range remotes { r.logHeader(fmt.Sprintf("Cloud upload: %s → %s [mode: %s]", job.Name, remote, job.Mode)) + // CleanupArchives deliberately uses the parent ctx, not the per-invocation + // jobCtx created inside runRcloneJob: a slow housekeeping pass shouldn't + // inherit the job's max_runtime and cause the cleanup to time out. if err := r.rclone.CleanupArchives(ctx, remote, job.BackupPath, job.BackupRetentionDays, r.dryRun); err != nil { r.logWarn(fmt.Sprintf("archive cleanup for %s: %v", remote, err)) } @@ -216,6 +220,40 @@ func collectErrors(jobs []JobResult) []string { return errs } +// classifyExitStatus maps the combination of a context and a command run error +// to the appropriate Status. Call after the command has terminated or failed +// to start (both cmd.Start and cmd.Wait error paths). +// +// When ctx.Err() is context.DeadlineExceeded the job's per-invocation deadline +// elapsed, so StatusTimedOut is returned regardless of runErr. When ctx.Err() +// is context.Canceled the parent was cancelled (e.g. by a signal), which is +// treated as an ordinary failure. context.Err() returns whichever terminal +// state the context reached first and stays there, so the "deadline first then +// parent cancel" case naturally resolves to StatusTimedOut without any extra +// ordering logic. +func classifyExitStatus(ctx context.Context, runErr error) Status { + if errors.Is(ctx.Err(), context.DeadlineExceeded) { + return StatusTimedOut + } + if runErr != nil { + return StatusFailed + } + return StatusOK +} + +// jobContext returns a context and cancel function for a single rsync or rclone +// invocation. When maxRuntime is zero the parent context is returned unchanged +// with a no-op cancel so callers can always defer cancel() safely. When +// maxRuntime is positive a child context with the corresponding deadline is +// returned; the caller is responsible for calling cancel to release the timer +// resource. +func jobContext(parent context.Context, maxRuntime time.Duration) (context.Context, context.CancelFunc) { + if maxRuntime <= 0 { + return parent, func() {} + } + return context.WithTimeout(parent, maxRuntime) +} + // runRsyncJob iterates each source in the job and calls rsync. func (r *Runner) runRsyncJob(ctx context.Context, job config.Job) JobResult { var items []ItemResult @@ -260,8 +298,16 @@ func (r *Runner) runRsyncJob(ctx context.Context, job config.Job) JobResult { args := BuildRsyncArgs(defaults, job, resolved, job.Destination, job.Delete && isDir, r.dryRun, r.logFile) r.logger.Debug(formatExec("rsync", args)) - r.pw.StartJob(ctx, label) - result := r.rsync.Exec(ctx, args, r.pw.ProgressCallback()) + // MaxRuntimeDuration returns (0, nil) for empty or (duration, nil) for + // validated input. The LoadBytes path rejects bad values at parse time, + // so discarding the error is safe here; callers bypassing validation + // (e.g. direct Job literals in tests) see that branch instead. + maxRuntime, _ := job.MaxRuntimeDuration() + jobCtx, cancel := jobContext(ctx, maxRuntime) + + r.pw.StartJob(jobCtx, label) + result := r.rsync.Exec(jobCtx, args, r.pw.ProgressCallback()) + cancel() r.pw.FinishJob(result) items = append(items, result) } @@ -328,8 +374,13 @@ func (r *Runner) runRcloneJob(ctx context.Context, job config.Job, remoteName, t args := BuildRcloneArgs(subcommand, rcloneDefaults, job, source, destination, r.dryRun, r.logFile, backupDirArg) r.logger.Debug(formatExec("rclone", args)) - r.pw.StartJob(ctx, label) - result := r.rclone.Exec(ctx, args, r.pw.ProgressCallback()) + // See the rsync branch for why the error is discarded; same reasoning applies. + maxRuntime, _ := job.MaxRuntimeDuration() + jobCtx, cancel := jobContext(ctx, maxRuntime) + defer cancel() + + r.pw.StartJob(jobCtx, label) + result := r.rclone.Exec(jobCtx, args, r.pw.ProgressCallback()) result.Name = destName if destName == "" { result.Name = "(prefix root)" diff --git a/internal/engine/runner_test.go b/internal/engine/runner_test.go index 6f35cc7..5ccf5bd 100644 --- a/internal/engine/runner_test.go +++ b/internal/engine/runner_test.go @@ -3,12 +3,15 @@ package engine import ( "bytes" "context" + "errors" + "fmt" "io" "os" "os/exec" "path/filepath" "strings" "testing" + "time" "github.com/jkleinne/shuttle/internal/config" "github.com/jkleinne/shuttle/internal/log" @@ -269,3 +272,203 @@ func TestRunRcloneJob_NotOptional_MissingLocalSource_MarksNotFound(t *testing.T) t.Errorf("Status = %q, want %q", result.Items[0].Status, StatusNotFound) } } + +func TestJobContext_NoTimeout_HasNoDeadline(t *testing.T) { + ctx, cancel := jobContext(context.Background(), 0) + defer cancel() + + if _, hasDeadline := ctx.Deadline(); hasDeadline { + t.Error("jobContext(parent, 0) must not set a deadline") + } +} + +func TestJobContext_WithTimeout_SetsDeadline(t *testing.T) { + const timeout = 1 * time.Hour + before := time.Now() + ctx, cancel := jobContext(context.Background(), timeout) + defer cancel() + + deadline, hasDeadline := ctx.Deadline() + if !hasDeadline { + t.Fatal("jobContext(parent, 1h) must set a deadline") + } + + // Deadline should be approximately 1 hour from now (allow 5s of drift). + wantMin := before.Add(timeout - 5*time.Second) + wantMax := before.Add(timeout + 5*time.Second) + if deadline.Before(wantMin) || deadline.After(wantMax) { + t.Errorf("deadline %v not in expected range [%v, %v]", deadline, wantMin, wantMax) + } +} + +func TestRunRsyncJob_MaxRuntime_FiresAndReportsTimedOut(t *testing.T) { + if _, err := exec.LookPath("rsync"); err != nil { + t.Skip("rsync not found on PATH") + } + + var termBuf bytes.Buffer + r := newTestRunner(t, &termBuf) + + src := t.TempDir() + // Seed enough files that rsync cannot finish in 1ms. + for i := range 20 { + name := filepath.Join(src, fmt.Sprintf("file%d.dat", i)) + if err := os.WriteFile(name, make([]byte, 1024*1024), 0o644); err != nil { + t.Fatalf("seeding source: %v", err) + } + } + + job := config.Job{ + Name: "timeout-job", + Engine: config.EngineRsync, + Sources: []string{src}, + Destination: t.TempDir(), + MaxRuntime: "1ms", + } + + result := r.runRsyncJob(context.Background(), job) + + if len(result.Items) != 1 { + t.Fatalf("Items count = %d, want 1", len(result.Items)) + } + if result.Items[0].Status != StatusTimedOut { + t.Errorf("Status = %q, want %q", result.Items[0].Status, StatusTimedOut) + } +} + +func TestRunRsyncJob_ParentCanceled_ReturnsFailedNotTimedOut(t *testing.T) { + if _, err := exec.LookPath("rsync"); err != nil { + t.Skip("rsync not found on PATH") + } + + var termBuf bytes.Buffer + r := newTestRunner(t, &termBuf) + + src := t.TempDir() + if err := os.WriteFile(filepath.Join(src, "file.txt"), []byte("data"), 0o644); err != nil { + t.Fatalf("seeding source: %v", err) + } + + // Pre-cancel the parent context before running the job. + parentCtx, parentCancel := context.WithCancel(context.Background()) + parentCancel() + + job := config.Job{ + Name: "canceled-job", + Engine: config.EngineRsync, + Sources: []string{src}, + Destination: t.TempDir(), + MaxRuntime: "1h", // long timeout so the cancellation comes from the parent + } + + result := r.runRsyncJob(parentCtx, job) + + if len(result.Items) != 1 { + t.Fatalf("Items count = %d, want 1", len(result.Items)) + } + if result.Items[0].Status != StatusFailed { + t.Errorf("Status = %q, want %q (parent cancel must not produce StatusTimedOut)", result.Items[0].Status, StatusFailed) + } +} + +func TestRunRcloneJob_MaxRuntime_FiresAndReportsTimedOut(t *testing.T) { + // Symmetric to TestRunRsyncJob_MaxRuntime_FiresAndReportsTimedOut: pins + // that the jobContext wiring at the rclone call site flows the deadline + // through to RcloneExecutor and the resulting context error is classified + // as StatusTimedOut. The executor-level rclone timeout test covers the + // classification in isolation; this one covers the runner-level wiring. + if _, err := exec.LookPath("rclone"); err != nil { + t.Skip("rclone not found on PATH") + } + + var termBuf bytes.Buffer + r := newTestRunner(t, &termBuf) + + src := t.TempDir() + if err := os.WriteFile(filepath.Join(src, "file.txt"), []byte("data"), 0o644); err != nil { + t.Fatalf("seeding source: %v", err) + } + + job := config.Job{ + Name: "timeout-rclone-job", + Engine: config.EngineRclone, + Source: src, + Remotes: []string{"any-remote"}, + Mode: config.ModeCopy, + // --config /dev/null avoids touching the developer's real rclone config + // during the test. The remote name is irrelevant because the 1ms + // deadline kills the process before rclone resolves the remote. + ExtraFlags: []string{"--config", "/dev/null"}, + MaxRuntime: "1ms", + } + + result := r.runRcloneJob(context.Background(), job, "any-remote", "2026-04-18_000000") + + if len(result.Items) != 1 { + t.Fatalf("Items count = %d, want 1", len(result.Items)) + } + if result.Items[0].Status != StatusTimedOut { + t.Errorf("Status = %q, want %q", result.Items[0].Status, StatusTimedOut) + } +} + +func TestClassifyExitStatus(t *testing.T) { + someErr := errors.New("command failed") + + // "deadline first then parent cancel": construct a context that has already + // exceeded its deadline, then cancel the parent. context.Err() returns + // whichever terminal state was reached first — DeadlineExceeded — and stays + // there regardless of the subsequent cancel. + parentCtx, parentCancel := context.WithCancel(context.Background()) + pastDeadline := time.Now().Add(-1 * time.Second) + deadlineFirstCtx, deadlineFirstCancel := context.WithDeadline(parentCtx, pastDeadline) + // Trigger the parent cancel so both conditions are true, but deadline was first. + parentCancel() + defer deadlineFirstCancel() + + tests := []struct { + name string + ctx context.Context + runErr error + want Status + }{ + { + name: "ok context, nil error", + ctx: context.Background(), + runErr: nil, + want: StatusOK, + }, + { + name: "ok context, non-nil error", + ctx: context.Background(), + runErr: someErr, + want: StatusFailed, + }, + { + name: "deadline exceeded context, non-nil error", + ctx: func() context.Context { c, cancel := context.WithDeadline(context.Background(), time.Now().Add(-1*time.Second)); t.Cleanup(cancel); return c }(), + runErr: someErr, + want: StatusTimedOut, + }, + { + name: "canceled context, non-nil error", + ctx: func() context.Context { c, cancel := context.WithCancel(context.Background()); cancel(); return c }(), + runErr: someErr, + want: StatusFailed, + }, + { + name: "deadline first then parent cancel", + ctx: deadlineFirstCtx, + runErr: someErr, + want: StatusTimedOut, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := classifyExitStatus(tt.ctx, tt.runErr) + if got != tt.want { + t.Errorf("classifyExitStatus(...) = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/internal/engine/stats.go b/internal/engine/stats.go index deac9b1..577e396 100644 --- a/internal/engine/stats.go +++ b/internal/engine/stats.go @@ -28,6 +28,10 @@ const ( // outcome: excluded from Summary.HasErrors() and rendered with its // own dedicated symbol and tally segment. StatusOptionalMissing Status = "optional_missing" + // StatusTimedOut means the job's per-invocation deadline elapsed before + // the external tool exited. It is a failure: Summary.HasErrors() returns + // true and the CLI exits with code 1. + StatusTimedOut Status = "timed_out" ) // IsFailure reports whether a status represents a real failure that should @@ -36,7 +40,7 @@ const ( // on what "failure" means. StatusOptionalMissing is deliberately excluded: // it is a user-opted-in outcome for detachable sources, not a failure. func (s Status) IsFailure() bool { - return s == StatusFailed || s == StatusNotFound + return s == StatusFailed || s == StatusNotFound || s == StatusTimedOut } // TransferStats holds the quantitative output of a single rsync or rclone run. diff --git a/internal/engine/stats_test.go b/internal/engine/stats_test.go index 89d02be..3c4348a 100644 --- a/internal/engine/stats_test.go +++ b/internal/engine/stats_test.go @@ -267,6 +267,14 @@ func TestSummary_HasErrors_ReturnsTrue_WhenOptionalMissingMixedWithFailed(t *tes } } +func TestStatus_IsFailure_TimedOut(t *testing.T) { + // StatusTimedOut represents a job killed by its deadline; it must count as + // a failure so that Summary.HasErrors drives a non-zero exit code. + if !StatusTimedOut.IsFailure() { + t.Error("StatusTimedOut.IsFailure() = false, want true") + } +} + func TestJobLabel(t *testing.T) { tests := []struct { name string