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
Report unknown blob figures for a snapshot whose manifest cannot be read (closes #272 )
2026-10-08 07:12:51 +00:00 · 2026-10-08 08:46:11 +02:00
9 changed files with 273 additions and 19 deletions
@@ -398,7 +398,8 @@ key is skipped with a warning and is not printed. If a listed
 orphaned blob figures are reported as unknown; `--json` gives them as
 `null`, lists the remote key of each unreadable manifest in
 `unreadable_manifests` and counts the manifests under skipped names in
-`skipped_manifest_count`.
+`skipped_manifest_count`. An unreadable manifest also leaves its
+snapshot's blob count and blob size unknown, `null` in `--json`.
 * `--json`: Output as JSON

 **`remote nuke`**: Delete every snapshot's metadata and every blob from the
@@ -22,6 +22,24 @@ 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
+  orphan figures were already unknown in that case, but the snapshot's
+  row still gave 0 blobs and 0 B, in the table and in `--json`. The row
+  now reads `unknown` and `--json` gives `null`. A directory with no
+  manifest still shows 0, since the orphan figures count its blobs as
+  orphaned.
+
 - 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
@@ -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(
@@ -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
@@ -181,8 +181,11 @@ type SnapshotMetadataInfo struct {
 	ManifestSize int64  `json:"manifest_size"`
 	DatabaseSize int64  `json:"database_size"`
 	TotalSize    int64  `json:"total_size"`
-	BlobCount    int    `json:"blob_count"`
-	BlobsSize    int64  `json:"blobs_size"`
+
+	// Both stay nil (null in the JSON) when the snapshot's manifest was
+	// listed but could not be read.
+	BlobCount *int   `json:"blob_count"`
+	BlobsSize *int64 `json:"blobs_size"`

 	// Set when the listing holds this snapshot's manifest.json.zst. A
 	// backup interrupted before its manifest upload leaves a directory
@@ -380,6 +383,10 @@ func (v *Vaultik) collectReferencedBlobsFromManifests(
 	for _, snapshotID := range snapshotIDs {
 		info := snapshotMetadata[snapshotID]
 		if !info.hasManifest {
+			// The orphan figures count this directory's blobs as
+			// orphaned, so it references none.
+			info.BlobCount, info.BlobsSize = new(int), new(int64)
+
 			continue
 		}

@@ -395,7 +402,7 @@ func (v *Vaultik) collectReferencedBlobsFromManifests(
 			continue
 		}

-		info.BlobCount = manifest.BlobCount
+		blobCount := manifest.BlobCount

 		var blobsSize int64

@@ -404,7 +411,8 @@ func (v *Vaultik) collectReferencedBlobsFromManifests(
 			blobsSize += blob.CompressedSize
 		}

-		info.BlobsSize = blobsSize
+		info.BlobCount = &blobCount
+		info.BlobsSize = &blobsSize
 	}

 	return referencedBlobs, unreadable
@@ -516,13 +524,23 @@ func (v *Vaultik) printRemoteInfoTable(result *RemoteInfoResult) {
 		v.stdoutf("%s", separator)

 		for _, info := range result.Snapshots {
+			blobCount := unknownText
+			if info.BlobCount != nil {
+				blobCount = humanize.Comma(int64(*info.BlobCount))
+			}
+
+			blobsSize := unknownText
+			if info.BlobsSize != nil {
+				blobsSize = ubytes(*info.BlobsSize)
+			}
+
 			v.stdoutf(rowFormat,
 				truncateString(info.SnapshotID, snapshotIDColWidth),
 				ubytes(info.ManifestSize),
 				ubytes(info.DatabaseSize),
 				ubytes(info.TotalSize),
-				humanize.Comma(int64(info.BlobCount)),
-				ubytes(info.BlobsSize),
+				blobCount,
+				blobsSize,
 			)
 		}

@@ -42,7 +42,7 @@ func TestTableCountForReportSurfacesReadFailure(t *testing.T) {
 	assert.Nil(t, missing, "a failed read is unknown, not a count")

 	// The rendered count for a failed read must say unknown, never 0.
-	assert.Equal(t, countUnknown, countText(missing))
+	assert.Equal(t, unknownText, countText(missing))
 	assert.NotEqual(t, "0", countText(missing))
 }

@@ -57,7 +57,7 @@ func TestCountTextDistinguishesEmptyFromUnknown(t *testing.T) {

 	assert.Equal(t, "0", countText(&zero))
 	assert.Equal(t, "7", countText(&seven))
-	assert.Equal(t, countUnknown, countText(nil))
+	assert.Equal(t, unknownText, countText(nil))
 }

 // TestCountDiffUnknownWhenEitherSideUnknown checks that a delta computed
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"strings"
 	"testing"
 	"time"

@@ -69,6 +70,74 @@ func TestRemoteInfo_UnreadableManifestLeavesOrphansUnknown(t *testing.T) {
 	assert.Equal(t, []any{unreadableKey}, doc["unreadable_manifests"])
 }

+// TestRemoteInfo_UnreadableManifestLeavesSnapshotBlobsUnknown checks
+// that the row of a snapshot whose manifest cannot be read gives its
+// blob count and blob size as unknown in the table and as null in
+// --json, not as 0. A directory without a manifest still shows 0: the
+// orphan figures count its blobs as orphaned, so it references none.
+func TestRemoteInfo_UnreadableManifestLeavesSnapshotBlobsUnknown(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	env := newListEnv(t)
+	readableKey := env.addRemote(t, listRemoteID,
+		time.Date(2026, 3, 2, 0, 0, 0, 0, time.UTC))
+
+	unreadableKey := snapshot.RemoteSnapshotKey(listLocalID)
+	require.NoError(t, env.store.Put(context.Background(),
+		"metadata/"+unreadableKey+"/manifest.json.zst",
+		bytes.NewReader([]byte("not a valid manifest"))))
+
+	noManifestKey := snapshot.RemoteSnapshotKey("testhost_home_2026-03-03T10:00:00Z")
+	require.NoError(t, env.store.Put(context.Background(),
+		"metadata/"+noManifestKey+"/db.zst.age",
+		bytes.NewReader([]byte("not a valid database"))))
+
+	require.NoError(t, env.v.RemoteInfo(false))
+
+	// The table truncates the remote key, so a row is found by a prefix.
+	wantUnknown := map[string]int{readableKey: 0, unreadableKey: 2, noManifestKey: 0}
+	for key, want := range wantUnknown {
+		var row string
+
+		for line := range strings.SplitSeq(env.stdout.String(), "\n") {
+			if strings.HasPrefix(line, key[:16]) {
+				row = line
+			}
+		}
+
+		require.NotEmpty(t, row, "no table row for %s", key)
+		assert.Equal(t, want, strings.Count(row, "unknown"), "row: %q", row)
+	}
+
+	env.stdout.Reset()
+	require.NoError(t, env.v.RemoteInfo(true))
+
+	var doc struct {
+		Snapshots []map[string]any `json:"snapshots"`
+	}
+
+	require.NoError(t, json.Unmarshal(env.stdout.Bytes(), &doc))
+	require.Len(t, doc.Snapshots, len(wantUnknown))
+
+	for _, entry := range doc.Snapshots {
+		switch entry["snapshot_id"] {
+		case readableKey:
+			assert.InDelta(t, 1, entry["blob_count"], 0)
+			assert.InDelta(t, fiveMegabytes, entry["blobs_size"], 0)
+		case noManifestKey:
+			assert.InDelta(t, 0, entry["blob_count"], 0)
+			assert.InDelta(t, 0, entry["blobs_size"], 0)
+		default:
+			assert.Equal(t, unreadableKey, entry["snapshot_id"])
+			assert.Contains(t, entry, "blob_count")
+			assert.Nil(t, entry["blob_count"])
+			assert.Contains(t, entry, "blobs_size")
+			assert.Nil(t, entry["blobs_size"])
+		}
+	}
+}
+
 // TestRemoteInfo_SkipsNonConformingMetadataName checks that a directory
 // under metadata/ whose name is not a remote key is left out of the
 // report, and that the orphan figures are unknown when it holds a
@@ -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))
 	}
@@ -1756,9 +1766,9 @@ func (v *Vaultik) PruneDatabase() (*PruneResult, error) {
 	return result, nil
 }

-// countUnknown is what a count reads as when its query could not be run,
-// distinct from "0", which means the table really was empty.
-const countUnknown = "unknown"
+// unknownText is what a count or size reads as when it could not be
+// determined, distinct from "0", which is a real zero.
+const unknownText = "unknown"

 // tableCountForReport returns the row count of a table for the prune
 // summary, or nil if the count could not be read. A read failure is
@@ -1795,7 +1805,7 @@ func countDiff(before, after *int64) *int64 {
 // one that could not be queried.
 func countText(count *int64) string {
 	if count == nil {
-		return countUnknown
+		return unknownText
 	}

 	return strconv.FormatInt(*count, 10)
@@ -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)
+		})
+	}
+}