From 79a73fa122fce43fa42153a54b0d0f9cffe46dc1 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Thu, 8 Oct 2026 10:12:10 +0200 Subject: [PATCH] Count a file a backup could not store as failed (closes #280) A file that phase 1 of a backup counted and phase 2 could not open, because it was unreadable under --skip-errors or removed in between, was added to the unchanged count while its size stayed in BytesScanned. The summary showed it as unchanged with its bytes backed up, and the snapshots row's file_count and total_size included it. The scanner now counts such a file in FilesFailed and takes its size out of BytesScanned. The summary's files line adds "N failed", and file_count leaves the file out. A directory phase 2 cannot record is not counted as failed, since phase 1 counts no directories; that case has no test. Model: opus-5-5 --- TODO.md | 9 ++ internal/snapshot/scanner.go | 21 ++- internal/snapshot/snapshot.go | 2 +- internal/vaultik/snapshot.go | 14 +- internal/vaultik/snapshot_failed_file_test.go | 123 ++++++++++++++++++ 5 files changed, 163 insertions(+), 6 deletions(-) create mode 100644 internal/vaultik/snapshot_failed_file_test.go diff --git a/TODO.md b/TODO.md index 97be9f1..2c8fffe 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,15 @@ the tag exists and is exercised; what is left is merging `next` to # Completed Steps +- 2026-10-08: Counted a file that a backup could not store as failed + ([issue #280](https://git.eeqj.de/sneak/vaultik/issues/280)). A file + that phase 1 counted and phase 2 could not open, because it was + unreadable under `--skip-errors` or removed in between, was reported + in the summary as unchanged with its bytes as backed up, and the + `snapshots` row's `file_count` and `total_size` included it. The + summary now counts it as failed, and its data total and the row leave + it out. + - 2026-10-08: Made `remote info` report a snapshot's blob count and blob size as unknown when its manifest cannot be read ([issue #272](https://git.eeqj.de/sneak/vaultik/issues/272)). The diff --git a/internal/snapshot/scanner.go b/internal/snapshot/scanner.go index 79d7e33..70641c0 100644 --- a/internal/snapshot/scanner.go +++ b/internal/snapshot/scanner.go @@ -133,10 +133,13 @@ type ScannerConfig struct { // ScanResult contains the results of a scan operation. Files and bytes // are counted per file: BytesScanned is the size of the new and changed -// files, BytesSkipped that of the unchanged ones. +// files, BytesSkipped that of the unchanged ones. FilesFailed counts the +// new and changed files that phase 2 could not store; FilesScanned +// includes them and BytesScanned does not. type ScanResult struct { FilesScanned int FilesSkipped int + FilesFailed int FilesDeleted int BytesScanned int64 BytesSkipped int64 @@ -1365,7 +1368,7 @@ func (s *Scanner) processFileWithErrorHandling( log.Warn("File was deleted during backup, skipping", "path", fileToProcess.Path) - result.FilesSkipped++ + countFailedFile(fileToProcess, result) return true, nil } @@ -1376,7 +1379,7 @@ func (s *Scanner) processFileWithErrorHandling( s.ui.Errorf("Failed to process %s: %v. Skipping (--skip-errors).", s.ui.Path(fileToProcess.Path), err) - result.FilesSkipped++ + countFailedFile(fileToProcess, result) return true, nil } @@ -1387,6 +1390,18 @@ func (s *Scanner) processFileWithErrorHandling( return false, nil } +// countFailedFile counts a file that phase 2 could not store as failed +// and takes its size back out of BytesScanned, where phase 1 put it. +// Phase 1 counts no directories, so a directory is not counted here. +func countFailedFile(fileToProcess *FileToProcess, result *ScanResult) { + if fileToProcess.FileInfo.IsDir() { + return + } + + result.FilesFailed++ + result.BytesScanned -= fileToProcess.FileInfo.Size() +} + // printProcessingProgress prints a periodic progress line during the process phase, // showing files processed, bytes transferred, throughput, and ETA func (s *Scanner) printProcessingProgress( diff --git a/internal/snapshot/snapshot.go b/internal/snapshot/snapshot.go index 05e1ac1..cdf641e 100644 --- a/internal/snapshot/snapshot.go +++ b/internal/snapshot/snapshot.go @@ -892,7 +892,7 @@ func (sm *SnapshotManager) getFileSize(path string) int64 { // BackupStats contains statistics from a backup operation type BackupStats struct { FilesScanned int - TotalSize int64 // Total size of all files examined + TotalSize int64 // Total size of the files in the snapshot ChunksCreated int BlobsCreated int BytesUploaded int64 diff --git a/internal/vaultik/snapshot.go b/internal/vaultik/snapshot.go index 0b14640..97ecfdb 100644 --- a/internal/vaultik/snapshot.go +++ b/internal/vaultik/snapshot.go @@ -185,6 +185,7 @@ type snapshotStats struct { totalBlobs int totalBytesSkipped int64 totalFilesSkipped int + totalFilesFailed int totalFilesDeleted int totalBytesDeleted int64 totalBytesUploaded int64 @@ -315,6 +316,7 @@ func (v *Vaultik) scanAllDirectories( stats.totalChunks += result.ChunksCreated stats.totalBlobs += result.BlobsCreated stats.totalFilesSkipped += result.FilesSkipped + stats.totalFilesFailed += result.FilesFailed stats.totalBytesSkipped += result.BytesSkipped stats.totalFilesDeleted += result.FilesDeleted stats.totalBytesDeleted += result.BytesDeleted @@ -326,6 +328,7 @@ func (v *Vaultik) scanAllDirectories( "path", dir, "files", result.FilesScanned, "files_skipped", result.FilesSkipped, + "files_failed", result.FilesFailed, "bytes", result.BytesScanned, "bytes_skipped", result.BytesSkipped, "chunks", result.ChunksCreated, @@ -359,9 +362,11 @@ func (v *Vaultik) finalizeSnapshotMetadata( return fmt.Errorf("getting snapshot blob sizes: %w", err) } + // file_count and total_size leave out the files that could not be + // stored; stats.totalBytes already does. extStats := snapshot.ExtendedBackupStats{ BackupStats: snapshot.BackupStats{ - FilesScanned: stats.totalFiles, + FilesScanned: stats.totalFiles - stats.totalFilesFailed, TotalSize: stats.totalBytes + stats.totalBytesSkipped, ChunksCreated: stats.totalChunks, BlobsCreated: stats.totalBlobs, @@ -409,7 +414,8 @@ func (v *Vaultik) printSnapshotSummary( snapshotID string, startTime time.Time, stats *snapshotStats, ) { snapshotDuration := time.Since(startTime) - totalFilesChanged := stats.totalFiles - stats.totalFilesSkipped + totalFilesChanged := stats.totalFiles - stats.totalFilesSkipped - + stats.totalFilesFailed totalBytesAll := stats.totalBytes + stats.totalBytesSkipped var compressionRatio float64 @@ -426,6 +432,10 @@ func (v *Vaultik) printSnapshotSummary( v.UI.Count(stats.totalFiles), v.UI.Count(totalFilesChanged), v.UI.Count(stats.totalFilesSkipped)) + if stats.totalFilesFailed > 0 { + filesMsg += fmt.Sprintf(", %s failed", v.UI.Count(stats.totalFilesFailed)) + } + if stats.totalFilesDeleted > 0 { filesMsg += fmt.Sprintf(", %s deleted", v.UI.Count(stats.totalFilesDeleted)) } diff --git a/internal/vaultik/snapshot_failed_file_test.go b/internal/vaultik/snapshot_failed_file_test.go new file mode 100644 index 0000000..a5f8bc4 --- /dev/null +++ b/internal/vaultik/snapshot_failed_file_test.go @@ -0,0 +1,123 @@ +package vaultik_test + +import ( + "bytes" + "context" + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/config" + "sneak.berlin/go/vaultik/internal/database" + "sneak.berlin/go/vaultik/internal/log" + "sneak.berlin/go/vaultik/internal/storage" + "sneak.berlin/go/vaultik/internal/ui" + "sneak.berlin/go/vaultik/internal/vaultik" +) + +// openFailFs is the real filesystem, except that opening path fails +// with err. Phase 1 of a backup only lstats a file, so it still counts +// path; phase 2 is the first to open it. +type openFailFs struct { + afero.OsFs + + path string + err error +} + +//nolint:ireturn // afero.Fs.Open is defined to return the interface. +func (f *openFailFs) Open(name string) (afero.File, error) { + if name == f.path { + return nil, &os.PathError{Op: "open", Path: name, Err: f.err} + } + + return f.OsFs.Open(name) +} + +// A file that phase 2 cannot open is reported as failed, not as +// unchanged, and neither the summary's data total nor the snapshots row +// counts it. See https://git.eeqj.de/sneak/vaultik/issues/280. +func TestSnapshotSummaryCountsFileNotStoredAsFailed(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + tests := []struct { + name string + openErr error + skipErrors bool + }{ + // What a normal user gets opening a file with mode 000. + {name: "unopenable under skip-errors", + openErr: os.ErrPermission, skipErrors: true}, + // What opening a file removed after phase 1 gives. + {name: "removed between the phases", + openErr: os.ErrNotExist, skipErrors: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + // The scan walks the source path with symlinks resolved, so + // failedPath must be spelled the same way to match. + tempDir, err := filepath.EvalSymlinks(t.TempDir()) + require.NoError(t, err) + + srcDir := filepath.Join(tempDir, "src") + failedPath := filepath.Join(srcDir, "failed.txt") + storedContent := []byte("this file is backed up") + storedSize := int64(len(storedContent)) + + fs := &openFailFs{path: failedPath, err: tt.openErr} + require.NoError(t, fs.MkdirAll(srcDir, 0o755)) + require.NoError(t, afero.WriteFile(fs, + filepath.Join(srcDir, "stored.txt"), storedContent, 0o644)) + require.NoError(t, afero.WriteFile(fs, + failedPath, []byte("this file cannot be opened"), 0o644)) + + cfg := faultTestConfig() + cfg.IndexPath = filepath.Join(tempDir, "index.sqlite") + cfg.Snapshots = map[string]config.SnapshotConfig{ + "src": {Paths: []string{srcDir}}, + } + + store, err := storage.NewFileStorer(filepath.Join(tempDir, "remote")) + require.NoError(t, err) + + db, err := database.New(ctx, cfg.IndexPath) + require.NoError(t, err) + t.Cleanup(func() { _ = db.Close() }) + + repos := database.NewRepositories(db) + out := &bytes.Buffer{} + v := newBackupVaultik(ctx, cfg, store, repos, db, fs) + v.UI = ui.NewWithColor(out, false) + + require.NoError(t, v.CreateSnapshot(&vaultik.SnapshotCreateOptions{ + SkipErrors: tt.skipErrors, + Snapshots: []string{"src"}, + })) + + summary := out.String() + assert.Contains(t, summary, + "Files: 2 examined, 1 backed up, 0 unchanged, 1 failed.") + assert.Contains(t, summary, + fmt.Sprintf("Data: %s total (%s backed up).", + v.UI.Size(storedSize), v.UI.Size(storedSize))) + + snap, err := repos.Snapshots.GetByID(ctx, + localSnapshotID(ctx, t, repos, "src")) + require.NoError(t, err) + require.NotNil(t, snap) + + assert.Equal(t, int64(1), snap.FileCount) + assert.Equal(t, storedSize, snap.TotalSize) + }) + } +}