Abort the run when packing fails, even under --skip-errors (closes #161)

A chunk is registered as pending (known, scanner-pending, packer pending-row) before it is packed. Under --skip-errors the scanner skipped a file on any processing error, including a failure inside addChunkToPacker (packing, database, encryption, upload). The pending chunk then stayed queued and a later blob finalize inserted it into the chunks table with no blob_chunks row, so a snapshot could complete holding a file whose chunk is in no blob and cannot be restored.

Errors from addChunkToPacker are now marked and abort the run regardless of --skip-errors; only open and read errors are skipped. The bookkeeping order is unchanged. Flag help and comments now say only unreadable files are skipped.

Model: opus-4-8

internal/snapshot/skip_errors_test.go

@@ -0,0 +1,216 @@
+package snapshot_test
+
+import (
+	"context"
+	"errors"
+	"os"
+	"path/filepath"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/spf13/afero"
+	"sneak.berlin/go/vaultik/internal/database"
+	"sneak.berlin/go/vaultik/internal/snapshot"
+)
+
+// errSimTempFail is the one-time temp-file creation failure blobTempFailFs
+// injects, mirroring a full temp filesystem.
+var errSimTempFail = errors.New("simulated temp-file creation failure")
+
+// errSimRead is the read failure readFailFile injects for a file that opens
+// but cannot be read.
+var errSimRead = errors.New("simulated read failure")
+
+// blobTempFailFs fails the first temp-file creation for a packer blob, then
+// behaves normally, simulating a one-time failure to start a new blob.
+type blobTempFailFs struct {
+	afero.Fs
+
+	mu     sync.Mutex
+	failed bool
+}
+
+//nolint:ireturn // afero.Fs.OpenFile is defined to return the interface.
+func (f *blobTempFailFs) OpenFile(
+	name string, flag int, perm os.FileMode,
+) (afero.File, error) {
+	if strings.Contains(name, "vaultik-blob-") {
+		f.mu.Lock()
+		firstTime := !f.failed
+		f.failed = true
+		f.mu.Unlock()
+
+		if firstTime {
+			return nil, errSimTempFail
+		}
+	}
+
+	return f.Fs.OpenFile(name, flag, perm)
+}
+
+// readFailFile wraps an afero.File whose Read always fails.
+type readFailFile struct {
+	afero.File
+}
+
+func (readFailFile) Read([]byte) (int, error) {
+	return 0, errSimRead
+}
+
+// readFailFs fails reads of one target path after a successful open.
+type readFailFs struct {
+	afero.Fs
+
+	target string
+}
+
+//nolint:ireturn // afero.Fs.Open is defined to return the interface.
+func (f *readFailFs) Open(name string) (afero.File, error) {
+	file, err := f.Fs.Open(name)
+	if err != nil {
+		return nil, err
+	}
+
+	if name == f.target {
+		return readFailFile{File: file}, nil
+	}
+
+	return file, nil
+}
+
+// writeSkipErrorTestFile writes one file into fs with a fixed mtime.
+func writeSkipErrorTestFile(t *testing.T, fs afero.Fs, path, content string) {
+	t.Helper()
+
+	err := fs.MkdirAll(filepath.Dir(path), 0755)
+	if err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+
+	err = afero.WriteFile(fs, path, []byte(content), 0644)
+	if err != nil {
+		t.Fatalf("write %s: %v", path, err)
+	}
+
+	when := time.Date(2024, 1, 1, 12, 0, 0, 0, time.UTC)
+
+	err = fs.Chtimes(path, when, when)
+	if err != nil {
+		t.Fatalf("chtimes %s: %v", path, err)
+	}
+}
+
+// runSkipErrorScan scans /source on fs with the given skip-errors setting and
+// returns the repositories (for inspection) and the scan error.
+func runSkipErrorScan(
+	t *testing.T, fs afero.Fs, skipErrors bool,
+) (*database.Repositories, error) {
+	t.Helper()
+
+	db, err := database.NewTestDB()
+	if err != nil {
+		t.Fatalf("create test db: %v", err)
+	}
+
+	t.Cleanup(func() {
+		cerr := db.Close()
+		if cerr != nil {
+			t.Errorf("close db: %v", cerr)
+		}
+	})
+
+	repos := database.NewRepositories(db)
+
+	scanner := snapshot.NewScanner(snapshot.ScannerConfig{
+		FS:               fs,
+		ChunkSize:        int64(1024 * 16),
+		Repositories:     repos,
+		MaxBlobSize:      int64(1024 * 1024),
+		CompressionLevel: 3,
+		AgeRecipients:    []string{testAgePublicKey},
+		SkipErrors:       skipErrors,
+	})
+
+	ctx := context.Background()
+	snapshotID := "test-snapshot-skip-errors"
+	createTestSnapshotRecord(ctx, t, repos, snapshotID)
+
+	_, err = scanner.Scan(ctx, "/source", snapshotID)
+
+	return repos, err
+}
+
+// TestScannerPackingFailureAbortsUnderSkipErrors checks that a failure to start
+// a new blob aborts the run even with --skip-errors. Otherwise the file would
+// be skipped while its chunk had already been registered as pending, letting a
+// later blob record that chunk in the chunks table with no blob to back it —
+// a snapshot that completes with a file that cannot be restored.
+func TestScannerPackingFailureAbortsUnderSkipErrors(t *testing.T) {
+	t.Parallel()
+
+	// Two files with distinct content so each yields a distinct chunk: the
+	// first fails to start a blob, and without the fix the second's blob would
+	// commit the first's orphaned chunk row.
+	fs := &blobTempFailFs{Fs: afero.NewMemMapFs()}
+	writeSkipErrorTestFile(t, fs, "/source/file1.txt", "first file content")
+	writeSkipErrorTestFile(t, fs, "/source/file2.txt", "second file content")
+
+	repos, err := runSkipErrorScan(t, fs, true)
+	if err == nil {
+		t.Fatal("expected scan to abort on the packer error, got nil")
+	}
+
+	// ListUnpacked returns chunks recorded with no blob_chunks row: exactly the
+	// unrestorable state this fix prevents.
+	unpacked, err := repos.Chunks.ListUnpacked(context.Background(), 10)
+	if err != nil {
+		t.Fatalf("listing unpacked chunks: %v", err)
+	}
+
+	if len(unpacked) != 0 {
+		t.Fatalf("expected no chunk recorded without a blob, got %d", len(unpacked))
+	}
+}
+
+// TestScannerReadErrorAbortsWithoutSkipErrors checks that a file read error
+// aborts the run when --skip-errors is not set.
+func TestScannerReadErrorAbortsWithoutSkipErrors(t *testing.T) {
+	t.Parallel()
+
+	const target = "/source/unreadable.txt"
+
+	fs := &readFailFs{Fs: afero.NewMemMapFs(), target: target}
+	writeSkipErrorTestFile(t, fs, target, "content that cannot be read")
+
+	_, err := runSkipErrorScan(t, fs, false)
+	if err == nil {
+		t.Fatal("expected scan to fail on the read error, got nil")
+	}
+}
+
+// TestScannerReadErrorSkippedWithSkipErrors checks that a file read error is
+// skipped and the run completes when --skip-errors is set.
+func TestScannerReadErrorSkippedWithSkipErrors(t *testing.T) {
+	t.Parallel()
+
+	const target = "/source/unreadable.txt"
+
+	fs := &readFailFs{Fs: afero.NewMemMapFs(), target: target}
+	writeSkipErrorTestFile(t, fs, target, "content that cannot be read")
+
+	repos, err := runSkipErrorScan(t, fs, true)
+	if err != nil {
+		t.Fatalf("expected scan to complete with --skip-errors, got %v", err)
+	}
+
+	chunks, err := repos.FileChunks.GetByFile(context.Background(), target)
+	if err != nil {
+		t.Fatalf("getting file chunks: %v", err)
+	}
+
+	if len(chunks) != 0 {
+		t.Fatalf("expected unreadable file skipped, got %d chunks", len(chunks))
+	}
+}