diff --git a/TODO.md b/TODO.md index 4e052b4..001f615 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: Kept the progress line of a snapshot with more than one path within 100% ([issue #271](https://git.eeqj.de/sneak/vaultik/issues/271)). 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 43ab3e1..5e923c5 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) + }) + } +}