Exit non-zero when snapshot create --skip-errors skips anything it cannot read (closes #310)

A backup under --skip-errors printed its skip errors and failed count,
then "Finished successfully.", and exited 0, so a cron wrapper could
not tell it from a complete backup. A directory the walk could not list
was not counted at all.

Each entry the walk skips is now counted as examined and failed. When
--skip-errors skipped anything it could not read or list, the run keeps
its snapshots, runs the post-backup prune as before, and then returns
an error the command reports as "Backup completed, but N file(s) could
not be read". A file removed during the run is still counted as failed
in the summary but leaves the exit status at 0, with or without the
flag.

Model: opus-5-5

README.md

@@ -178,7 +178,7 @@ vaultik version
 * `--verbose`, `-v`: Enable verbose output (on stderr — see below)
 * `--debug`: Enable debug output (on stderr — see below)
 * `--quiet`, `-q`: Suppress non-error output (also suppresses startup banner)
-* `--skip-errors`: Skip files that cannot be read when creating a snapshot, or that cannot be restored when restoring, instead of aborting. Packing and storage errors while creating a snapshot (which would leave a chunk recorded but not stored) still abort the run. A run that skipped anything still finishes but exits non-zero: `snapshot create` keeps the snapshot and counts each file or directory it skipped as failed.
+* `--skip-errors`: Skip files that cannot be read when creating a snapshot, or that cannot be restored when restoring, instead of aborting. Packing and storage errors while creating a snapshot (which would leave a chunk recorded but not stored) still abort the run. A run that skipped anything still finishes but exits non-zero: `snapshot create` keeps the snapshot and counts each file or directory it skipped as failed. A file removed during the backup is also counted as failed but does not change the exit status.
 
 ### locking
 

internal/cli/entry_snapshot_create_skip_errors_test.go

@@ -25,6 +25,6 @@ func TestEntrySnapshotCreateSkipErrorsExitsNonZeroWhenItSkips(t *testing.T) {
 		"Files: 1 examined, 0 backed up, 0 unchanged, 1 failed.")
 	assert.NotContains(t, stdout, "Finished successfully.")
 	assert.Contains(t, stderr,
-		"Backup completed, but 1 file(s) failed to back up")
+		"Backup completed, but 1 file(s) could not be read")
 	assert.NotContains(t, stderr, "Snapshot creation failed")
 }

internal/cli/snapshot.go

@@ -115,8 +115,8 @@ on Linux), then /etc/vaultik/config.yml.`,
 					return
 				}
 
-				if errors.Is(err, vaultik.ErrFilesFailedBackup) {
-					log.Error("Backup completed with failed files", "error", err)
+				if errors.Is(err, vaultik.ErrFilesUnreadable) {
+					log.Error("Backup completed with unreadable files", "error", err)
 					ReportErrorf("Backup completed, but %v", err)
 
 					return

internal/snapshot/scanner.go

@@ -162,6 +162,11 @@ type ScanResult struct {
 	UploadDuration time.Duration
 	StartTime      time.Time
 	EndTime        time.Time
+
+	// FilesUnreadable is the part of FilesFailed that --skip-errors
+	// skipped because it could not be read or listed; the rest are files
+	// removed after the walk listed them.
+	FilesUnreadable int
 }
 
 // NewScanner creates a new scanner instance
@@ -985,7 +990,7 @@ func (s *Scanner) recordSpecialEntry(
 
 // handleWalkError deals with a filesystem error surfaced by the walk:
 // under --skip-errors the entry is skipped with a UI notice and counted
-// as examined and failed, otherwise the error is fatal.
+// as examined, failed and unreadable, otherwise the error is fatal.
 func (s *Scanner) handleWalkError(
 	filePath string, err error, result *ScanResult,
 ) error {
@@ -997,6 +1002,7 @@ func (s *Scanner) handleWalkError(
 
 		result.FilesScanned++
 		result.FilesFailed++
+		result.FilesUnreadable++
 
 		return nil // Continue scanning
 	}
@@ -1429,6 +1435,7 @@ func (s *Scanner) processFileWithErrorHandling(
 				s.ui.Path(fileToProcess.Path), err)
 
 			countFailedFile(fileToProcess, result)
+			result.FilesUnreadable++
 
 			return true, nil
 		}

internal/vaultik/snapshot.go

@@ -39,10 +39,10 @@ var (
 // caller can report it apart from a failed backup.
 var ErrPostBackupPruneFailed = errors.New("post-backup prune failed")
 
-// ErrFilesFailedBackup is returned when --skip-errors skipped files or
-// directories. The snapshot is complete and kept, so the caller can
-// report it apart from a failed backup.
-var ErrFilesFailedBackup = errors.New("file(s) failed to back up")
+// ErrFilesUnreadable is returned when --skip-errors skipped files or
+// directories it could not read or list. The snapshot is complete and
+// kept, so the caller can report it apart from a failed backup.
+var ErrFilesUnreadable = errors.New("file(s) could not be read")
 
 // listRecentLimit caps how many snapshot rows are fetched from the
 // local index for listing and purge operations.
@@ -140,16 +140,16 @@ func (v *Vaultik) CreateSnapshot(opts *SnapshotCreateOptions) error {
 	}
 
 	// Process each named snapshot
-	filesFailed := 0
+	filesUnreadable := 0
 
 	for snapIdx, snapName := range snapshotNames {
-		failed, err := v.createNamedSnapshot(
+		unreadable, err := v.createNamedSnapshot(
 			opts, hostname, snapName, snapIdx+1, len(snapshotNames))
 		if err != nil {
 			return err
 		}
 
-		filesFailed += failed
+		filesUnreadable += unreadable
 	}
 
 	// Print overall summary if multiple snapshots
@@ -165,17 +165,17 @@ func (v *Vaultik) CreateSnapshot(opts *SnapshotCreateOptions) error {
 		}
 	}
 
-	return v.finishCreateSnapshot(opts.SkipErrors, filesFailed)
+	return v.finishCreateSnapshot(filesUnreadable)
 }
 
 // finishCreateSnapshot ends a run whose snapshots are all complete and
 // kept. Like finishRestore, it returns an error when --skip-errors
-// skipped anything; otherwise it prints the closing line.
-func (v *Vaultik) finishCreateSnapshot(skipErrors bool, filesFailed int) error {
-	// Without --skip-errors a file is counted as failed only when it was
-	// removed during the run, which is not an error.
-	if skipErrors && filesFailed > 0 {
-		return fmt.Errorf("%d %w", filesFailed, ErrFilesFailedBackup)
+// skipped anything it could not read or list; otherwise it prints the
+// closing line. A file removed during the run is counted as failed in
+// the summary but is not an error, with or without --skip-errors.
+func (v *Vaultik) finishCreateSnapshot(filesUnreadable int) error {
+	if filesUnreadable > 0 {
+		return fmt.Errorf("%d %w", filesUnreadable, ErrFilesUnreadable)
 	}
 
 	// Terminus must obey the --cron invariant: silent on total
@@ -245,6 +245,10 @@ type snapshotStats struct {
 	totalBlobsUploaded int
 	uploadDuration     time.Duration
 
+	// The part of totalFilesFailed that --skip-errors skipped because it
+	// could not be read or listed.
+	totalFilesUnreadable int
+
 	// The sizes of all blobs the snapshot references, set by
 	// finalizeSnapshotMetadata once snapshot_blobs is populated.
 	blobSize             int64
@@ -252,7 +256,8 @@ type snapshotStats struct {
 }
 
 // createNamedSnapshot creates a single named snapshot and returns the
-// number of files counted as failed.
+// number of entries --skip-errors skipped because it could not read or
+// list them.
 func (v *Vaultik) createNamedSnapshot(
 	opts *SnapshotCreateOptions, hostname, snapName string, idx, total int,
 ) (int, error) {
@@ -310,7 +315,7 @@ func (v *Vaultik) createNamedSnapshot(
 
 	v.printSnapshotSummary(snapshotID, snapshotStartTime, stats)
 
-	return stats.totalFilesFailed, nil
+	return stats.totalFilesUnreadable, nil
 }
 
 // previousSnapshotID returns the ID of the most recent completed snapshot
@@ -396,6 +401,7 @@ func (v *Vaultik) scanAllDirectories(
 		stats.totalBlobs += result.BlobsCreated
 		stats.totalFilesSkipped += result.FilesSkipped
 		stats.totalFilesFailed += result.FilesFailed
+		stats.totalFilesUnreadable += result.FilesUnreadable
 		stats.totalBytesSkipped += result.BytesSkipped
 		stats.totalFilesDeleted += result.FilesDeleted
 		stats.totalBytesDeleted += result.BytesDeleted

internal/vaultik/snapshot_deleted_count_test.go

@@ -182,7 +182,7 @@ func TestBackupDoesNotCountFileInUnopenableDirectoryAsDeleted(t *testing.T) {
 		SkipErrors: true,
 		Snapshots:  []string{"work"},
 	})
-	require.ErrorIs(t, err, vaultik.ErrFilesFailedBackup)
+	require.ErrorIs(t, err, vaultik.ErrFilesUnreadable)
 	assert.Contains(t, env.out.String(),
 		"Files: 2 examined, 0 backed up, 1 unchanged, 1 failed.")
 }

internal/vaultik/snapshot_failed_file_test.go

@@ -54,10 +54,14 @@ func TestSnapshotSummaryCountsFileNotStoredAsFailed(t *testing.T) {
 		// What a normal user gets opening a file with mode 000.
 		{name: "unopenable under skip-errors",
 			openErr: os.ErrPermission, skipErrors: true,
-			wantErr: vaultik.ErrFilesFailedBackup},
+			wantErr: vaultik.ErrFilesUnreadable},
 		// What opening a file removed after phase 1 gives.
 		{name: "removed between the phases",
 			openErr: os.ErrNotExist, skipErrors: false},
+		// A removal is not an error, so --skip-errors does not make the
+		// run fail. See https://git.eeqj.de/sneak/vaultik/issues/310.
+		{name: "removed between the phases under skip-errors",
+			openErr: os.ErrNotExist, skipErrors: true},
 	}
 
 	for _, tt := range tests {

internal/vaultik/snapshot_skip_errors_test.go

@@ -112,7 +112,7 @@ func TestSkipErrorsBackupKeepsSnapshotAndFailsWhenItSkips(t *testing.T) {
 				SkipErrors: true,
 				Snapshots:  []string{"work"},
 			})
-			require.ErrorIs(t, err, vaultik.ErrFilesFailedBackup)
+			require.ErrorIs(t, err, vaultik.ErrFilesUnreadable)
 
 			assert.Contains(t, env.out.String(),
 				"Files: 2 examined, 1 backed up, 0 unchanged, 1 failed.")