From 58cdef451b165e2ba797e43693c9723821a34f09 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 8 Oct 2026 11:28:56 +0000 Subject: [PATCH] Stop a cancelled backup at once under --skip-errors (closes #286) After Ctrl-C or SIGTERM, phase 2 of a --skip-errors backup treated the cancellation error from each remaining file like an unreadable file: it opened the file, printed an error line, counted it as failed and moved on to the next. The processing loop now checks for cancellation before each file, and an error from a file once the run is cancelled stops the run instead of being skipped. The loop check is what stops a run cancelled while an empty file is open, since an empty file has no chunks and nothing reads the context while it is backed up. The --skip-errors test helper now takes the scan's context. Model: opus-5-5 --- TODO.md | 8 ++ internal/snapshot/scanner.go | 12 +++ internal/snapshot/skip_errors_test.go | 101 +++++++++++++++++++++++--- 3 files changed, 110 insertions(+), 11 deletions(-) diff --git a/TODO.md b/TODO.md index 46585b4..aab8ceb 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,14 @@ the tag exists and is exercised; what is left is merging `next` to # Completed Steps +- 2026-10-08: Made a backup cancelled under `--skip-errors` stop at once + ([issue #286](https://git.eeqj.de/sneak/vaultik/issues/286)). After + Ctrl-C or SIGTERM, phase 2 treated the cancellation error from each + remaining file like an unreadable file: it opened the file, printed an + error line for it, counted it as failed and went on to the next one. + The run now stops at the first file after the cancellation, and no + file is counted as failed because of it. + - 2026-10-08: Made `remote nuke` delete the `.partial` files that uploads cut off part-way leave on the destination store ([issue #281](https://git.eeqj.de/sneak/vaultik/issues/281)). The diff --git a/internal/snapshot/scanner.go b/internal/snapshot/scanner.go index 4527f69..c810278 100644 --- a/internal/snapshot/scanner.go +++ b/internal/snapshot/scanner.go @@ -1312,6 +1312,13 @@ func (s *Scanner) processPhase( // Process each file for _, fileToProcess := range filesToProcess { + // Check context cancellation + select { + case <-ctx.Done(): + return ctx.Err() + default: + } + // Update progress if s.progress != nil { s.progress.GetStats().CurrentFile.Store(fileToProcess.Path) @@ -1369,6 +1376,11 @@ func (s *Scanner) processFileWithErrorHandling( err := s.processFileStreaming(ctx, fileToProcess, result) if err != nil { + // A cancelled run stops here even under --skip-errors, rather than + // counting this file as failed and going on to the next one. + if ctx.Err() != nil { + return false, fmt.Errorf("processing file %s: %w", fileToProcess.Path, err) + } // A packer/database/encryption/upload failure means the chunk's data // may not have been stored. Skipping the file would let the snapshot // record a file whose chunk is in no blob and cannot be restored, so diff --git a/internal/snapshot/skip_errors_test.go b/internal/snapshot/skip_errors_test.go index 8325372..04ad8de 100644 --- a/internal/snapshot/skip_errors_test.go +++ b/internal/snapshot/skip_errors_test.go @@ -82,6 +82,32 @@ func (f *readFailFs) Open(name string) (afero.File, error) { return file, nil } +// cancelOnOpenFs cancels the run when the scanner opens the target file to +// back it up, as Ctrl-C partway through processing would, and records each +// file opened after that. +type cancelOnOpenFs struct { + afero.Fs + + target string + cancel context.CancelFunc + cancelled bool + openedAfterCancel []string +} + +//nolint:ireturn // afero.Fs.Open is defined to return the interface. +func (f *cancelOnOpenFs) Open(name string) (afero.File, error) { + if f.cancelled { + f.openedAfterCancel = append(f.openedAfterCancel, name) + } + + if name == f.target { + f.cancel() + f.cancelled = true + } + + return f.Fs.Open(name) +} + // linkRemovedAfterLstatFs is the real filesystem, except that the symlink at // target is removed right after the walk lstats it, as happens when a link is // deleted during a backup. The scanner's readlink of it then fails. @@ -128,15 +154,16 @@ func writeSkipErrorTestFile(t *testing.T, fs afero.Fs, path, content string) { } } -// runSkipErrorScan scans source on fs with the given skip-errors setting, -// printing user-facing messages to uiw (nil discards them), and returns the -// repositories (for inspection) and the scan error. +// runSkipErrorScan scans source on fs under ctx with the given skip-errors +// setting, printing user-facing messages to uiw (nil discards them), and +// returns the repositories (for inspection) and the scan error. func runSkipErrorScan( - t *testing.T, fs afero.Fs, source string, skipErrors bool, uiw *ui.Writer, + ctx context.Context, t *testing.T, fs afero.Fs, source string, + skipErrors bool, uiw *ui.Writer, ) (*database.Repositories, error) { t.Helper() - db, err := database.NewTestDB() + db, err := database.New(ctx, ":memory:") if err != nil { t.Fatalf("create test db: %v", err) } @@ -161,7 +188,6 @@ func runSkipErrorScan( SkipErrors: skipErrors, }) - ctx := context.Background() snapshotID := "test-snapshot-skip-errors" createTestSnapshotRecord(ctx, t, repos, snapshotID) @@ -185,7 +211,7 @@ func TestScannerPackingFailureAbortsUnderSkipErrors(t *testing.T) { writeSkipErrorTestFile(t, fs, "/source/file1.txt", "first file content") writeSkipErrorTestFile(t, fs, "/source/file2.txt", "second file content") - repos, err := runSkipErrorScan(t, fs, "/source", true, nil) + repos, err := runSkipErrorScan(context.Background(), t, fs, "/source", true, nil) if err == nil { t.Fatal("expected scan to abort on the packer error, got nil") } @@ -212,7 +238,7 @@ func TestScannerReadErrorAbortsWithoutSkipErrors(t *testing.T) { fs := &readFailFs{Fs: afero.NewMemMapFs(), target: target} writeSkipErrorTestFile(t, fs, target, "content that cannot be read") - _, err := runSkipErrorScan(t, fs, "/source", false, nil) + _, err := runSkipErrorScan(context.Background(), t, fs, "/source", false, nil) if err == nil { t.Fatal("expected scan to fail on the read error, got nil") } @@ -228,7 +254,7 @@ func TestScannerReadErrorSkippedWithSkipErrors(t *testing.T) { fs := &readFailFs{Fs: afero.NewMemMapFs(), target: target} writeSkipErrorTestFile(t, fs, target, "content that cannot be read") - repos, err := runSkipErrorScan(t, fs, "/source", true, nil) + repos, err := runSkipErrorScan(context.Background(), t, fs, "/source", true, nil) if err != nil { t.Fatalf("expected scan to complete with --skip-errors, got %v", err) } @@ -267,7 +293,7 @@ func TestScannerUnreadableSymlinkAbortsWithoutSkipErrors(t *testing.T) { sourceDir, linkPath := writeSymlinkSource(t) fs := &linkRemovedAfterLstatFs{t: t, target: linkPath} - _, err := runSkipErrorScan(t, fs, sourceDir, false, nil) + _, err := runSkipErrorScan(context.Background(), t, fs, sourceDir, false, nil) if !errors.Is(err, os.ErrNotExist) { t.Fatalf("expected scan to fail on the removed symlink, got %v", err) } @@ -283,7 +309,7 @@ func TestScannerUnreadableSymlinkSkippedWithSkipErrors(t *testing.T) { fs := &linkRemovedAfterLstatFs{t: t, target: linkPath} uiw := ui.NewWithColor(io.Discard, false) - repos, err := runSkipErrorScan(t, fs, sourceDir, true, uiw) + repos, err := runSkipErrorScan(context.Background(), t, fs, sourceDir, true, uiw) if err != nil { t.Fatalf("expected scan to complete with --skip-errors, got %v", err) } @@ -302,3 +328,56 @@ func TestScannerUnreadableSymlinkSkippedWithSkipErrors(t *testing.T) { t.Fatalf("expected %s not to be recorded", linkPath) } } + +// TestScannerCancelStopsSkipErrorsRun checks that a --skip-errors backup +// cancelled partway through processing returns the cancellation error, +// opens no further file, and reports no file as failed. +func TestScannerCancelStopsSkipErrorsRun(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + targetContent string + }{ + // The cancellation lands while the target is being read. + {name: "while reading a file", targetContent: "first file content"}, + // An empty target has no chunks, so the cancellation goes unnoticed + // until the run moves on to the next file. + {name: "between files", targetContent: ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + const target = "/source/a.txt" + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + fs := &cancelOnOpenFs{ + Fs: afero.NewMemMapFs(), target: target, cancel: cancel, + } + writeSkipErrorTestFile(t, fs, target, tt.targetContent) + writeSkipErrorTestFile(t, fs, "/source/b.txt", "second file content") + writeSkipErrorTestFile(t, fs, "/source/c.txt", "third file content") + + uiw := ui.NewWithColor(io.Discard, false) + + _, err := runSkipErrorScan(ctx, t, fs, "/source", true, uiw) + if !errors.Is(err, context.Canceled) { + t.Fatalf("expected the cancellation error, got %v", err) + } + + if len(fs.openedAfterCancel) != 0 { + t.Fatalf("expected no file opened after the cancellation, got %v", + fs.openedAfterCancel) + } + + if uiw.ErrorCount() != 0 { + t.Fatalf("expected no file reported as failed, got %d error lines", + uiw.ErrorCount()) + } + }) + } +} -- 2.54.0