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()) + } + }) + } +}