fix(backup): classify temp write failures correctly

RELEASE_NOTES_2026.09.2.md

@@ -324,6 +324,13 @@ fails the build rather than leaving the note quietly wrong.
 
 ## Bug fixes
 
+### Backup no longer skips temp-file write failures ([#779](https://github.com/Basekick-Labs/arc/issues/779))
+
+Backup now treats failures writing its temporary file as fatal instead of
+misclassifying them as unreadable source files and counting them as skipped.
+
+Contributed by [@efegokdemir](https://github.com/efegokdemir) in [#784](https://github.com/Basekick-Labs/arc/pull/784).
+
 ### Remaining DuckDB string-literal escaping uses the shared helper ([#781](https://github.com/Basekick-Labs/arc/issues/781))
 
 The remaining compaction and database DuckDB string-literal escaping now uses

internal/backup/backup.go

@@ -500,12 +500,8 @@ func (m *Manager) checkSkipRatio(progress *Progress, totalFiles int) error {
 // streamBackupFile streams a file from data storage to backup storage via a temp file,
 // avoiding loading the entire file into memory (important for large Parquet files).
 // It returns the number of bytes actually copied.
-//
-// Only a source-read failure is wrapped with errBackupRead (making it skippable by
-// the caller); temp file, seek, and write failures are returned unwrapped and are
-// fatal to the backup.
 func (m *Manager) streamBackupFile(ctx context.Context, srcPath, destPath string) (int64, error) {
-	tmpFile, err := os.CreateTemp("", "arc-backup-*.parquet")
+	tmpFile, err := createTempFile("arc-backup-*.parquet")
 	if err != nil {
 		return 0, fmt.Errorf("failed to create temp file: %w", err)
 	}
@@ -514,8 +510,9 @@ func (m *Manager) streamBackupFile(ctx context.Context, srcPath, destPath string
 	defer tmpFile.Close()
 
 	// Stream from data storage to temp file
-	if err := m.dataStorage.ReadTo(ctx, srcPath, tmpFile); err != nil {
-		return 0, fmt.Errorf("failed to read from data storage: %w: %w", errBackupRead, err)
+	tw := &trackingWriter{w: tmpFile}
+	if err := m.dataStorage.ReadTo(ctx, srcPath, tw); err != nil {
+		return 0, classifyReadToFailure(srcPath, err, tw.err, errBackupRead, "data storage")
 	}
 
 	// Size the upload from the temp file rather than the listing: the listing is a

internal/backup/backup_test.go

@@ -335,6 +335,47 @@ func TestCopyDataFiles_TempFileFailureIsFatal(t *testing.T) {
 	}
 }
 
+// A ReadTo failure caused by the temp file after it was created is fatal, not
+// a skipped source file. The temp file is opened read-only so its writes fail.
+func TestCopyDataFiles_TempWriteFailureIsFatalNotSkipped(t *testing.T) {
+	ctx := context.Background()
+	logger := zerolog.Nop()
+
+	dataStorage, err := storage.NewLocalBackend(t.TempDir(), logger)
+	if err != nil {
+		t.Fatalf("failed to create data storage: %v", err)
+	}
+	backupStorage := mustLocalBackend(t, t.TempDir(), logger)
+	m := &Manager{dataStorage: dataStorage, backupStorage: backupStorage, logger: logger}
+
+	srcPath := "db/cpu/2026/07/28/00/a.parquet"
+	if err := dataStorage.Write(ctx, srcPath, bytes.Repeat([]byte("z"), 10)); err != nil {
+		t.Fatalf("failed to write source: %v", err)
+	}
+
+	orig := createTempFile
+	t.Cleanup(func() { createTempFile = orig })
+	createTempFile = func(pattern string) (*os.File, error) {
+		p := filepath.Join(t.TempDir(), "ro.parquet")
+		if err := os.WriteFile(p, nil, 0o600); err != nil {
+			return nil, err
+		}
+		return os.OpenFile(p, os.O_RDONLY, 0)
+	}
+
+	progress := &Progress{Operation: "backup", TotalFiles: 1}
+	err = m.copyDataFiles(ctx, "bkid", []storage.ObjectInfo{{Path: srcPath, Size: 10}}, progress)
+	if err == nil {
+		t.Fatal("expected fatal temp-write failure")
+	}
+	if isSourceReadError(err) {
+		t.Errorf("temp-write failure must not be classified as a source read: %v", err)
+	}
+	if progress.SkippedFiles != 0 {
+		t.Errorf("SkippedFiles = %d, want 0", progress.SkippedFiles)
+	}
+}
+
 // A backup whose every file is unreadable must fail rather than silently
 // producing an empty backup that reports success. (100% skipped also exceeds
 // maxSkipRatio, but this pins the total-loss case explicitly.)

internal/backup/readto.go

@@ -0,0 +1,48 @@
+package backup
+
+import (
+	"fmt"
+	"io"
+	"os"
+)
+
+// createTempFile creates a per-file staging file. The caller supplies the
+// operation-specific pattern so backup and restore retain distinct temp-file
+// names while sharing the test seam.
+//
+// Wrapping the returned *os.File in trackingWriter costs the source-to-temp
+// io.Copy's zero-copy fast path (copy_file_range needs an *os.File
+// destination), so that hop runs through a buffered loop. This is deliberate:
+// the operation is disk-bound, and the alternative is not knowing whether a
+// ReadTo failure came from the source or the local temp destination.
+var createTempFile = func(pattern string) (*os.File, error) {
+	return os.CreateTemp("", pattern)
+}
+
+// trackingWriter records the first error the destination returned.
+//
+// Every backend's ReadTo is an io.Copy into the caller's writer, so a full temp
+// filesystem surfaces as a ReadTo error that is indistinguishable, by the error
+// alone, from the source being unreadable. The recorded error lets
+// classifyReadToFailure attribute the failure to the right side.
+type trackingWriter struct {
+	w   io.Writer
+	err error
+}
+
+func (t *trackingWriter) Write(p []byte) (int, error) {
+	n, err := t.w.Write(p)
+	if err != nil && t.err == nil {
+		t.err = err
+	}
+	return n, err
+}
+
+// classifyReadToFailure turns a ReadTo failure into a destination error or a
+// caller-specific source-read error.
+func classifyReadToFailure(srcPath string, readErr, writeErr, sourceReadErr error, sourceName string) error {
+	if writeErr != nil {
+		return fmt.Errorf("failed to write temp file while reading %s from %s: %w", srcPath, sourceName, writeErr)
+	}
+	return fmt.Errorf("failed to read from %s: %w: %w", sourceName, sourceReadErr, readErr)
+}

internal/backup/restore.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"io"
 	"os"
 	"path/filepath"
 	"strings"
@@ -47,46 +46,9 @@ func isRestoreReadError(err error) bool {
 	return errors.Is(err, errRestoreRead)
 }
 
-// trackingWriter records the first error the destination returned.
-//
-// Every backend's ReadTo is an io.Copy into the caller's writer, so a full temp
-// filesystem surfaces as a ReadTo error that is indistinguishable, by the error
-// alone, from the source being unreadable. The recorded error lets
-// classifyReadTo attribute the failure to the right side.
-//
-// Wrapping the temp file costs the backup→temp hop io.Copy's zero-copy fast
-// path (copy_file_range needs an *os.File destination), so that hop runs
-// through a buffered loop. Deliberate: a restore is rare and disk-bound, and
-// the alternative is not knowing which side failed. The temp→data hop is
-// unaffected.
-type trackingWriter struct {
-	w   io.Writer
-	err error
-}
-
-func (t *trackingWriter) Write(p []byte) (int, error) {
-	n, err := t.w.Write(p)
-	if err != nil && t.err == nil {
-		t.err = err
-	}
-	return n, err
-}
-
-// createRestoreTemp creates the per-file staging temp file. A variable so tests
-// can hand streamRestoreFile a file that refuses writes, which is the only
-// portable way to drive the destination-side ReadTo failure end to end.
-var createRestoreTemp = func() (*os.File, error) {
-	return os.CreateTemp("", "arc-restore-*.parquet")
-}
-
-// classifyReadTo turns a ReadTo failure into the restore's two error classes: a
-// destination error recorded by the tracking writer is fatal; anything else is
-// a source read, wrapped with errRestoreRead so the caller can skip it.
+// classifyReadTo preserves restore's source-read and destination error wording.
 func classifyReadTo(srcPath string, readErr, writeErr error) error {
-	if writeErr != nil {
-		return fmt.Errorf("failed to write temp file while reading %s from backup: %w", srcPath, writeErr)
-	}
-	return fmt.Errorf("failed to read from backup: %w: %w", errRestoreRead, readErr)
+	return classifyReadToFailure(srcPath, readErr, writeErr, errRestoreRead, "backup")
 }
 
 // RestoreBackup restores data from a backup. It runs synchronously; the API
@@ -359,7 +321,7 @@ func (m *Manager) restoreDataFiles(ctx context.Context, backupID string, manifes
 // returned unwrapped and are fatal to the restore. A ReadTo failure caused by
 // the temp file itself (see trackingWriter) is fatal, not a read.
 func (m *Manager) streamRestoreFile(ctx context.Context, srcPath, destPath string) (int64, error) {
-	tmpFile, err := createRestoreTemp()
+	tmpFile, err := createTempFile("arc-restore-*.parquet")
 	if err != nil {
 		return 0, fmt.Errorf("failed to create temp file: %w", err)
 	}

internal/backup/restore_incomplete_fields_test.go

@@ -296,9 +296,9 @@ func TestRestore_MetadataSkipsDoNotMaskMissingDataFiles(t *testing.T) {
 // not a skipped object. The temp file is opened read-only so its writes fail.
 func TestRestore_TempWriteFailureIsFatalNotSkipped(t *testing.T) {
 	backupDir, backupID, _ := makeRestorableBackup(t, 3)
-	orig := createRestoreTemp
-	t.Cleanup(func() { createRestoreTemp = orig })
-	createRestoreTemp = func() (*os.File, error) {
+	orig := createTempFile
+	t.Cleanup(func() { createTempFile = orig })
+	createTempFile = func(pattern string) (*os.File, error) {
 		p := filepath.Join(t.TempDir(), "ro.parquet")
 		if err := os.WriteFile(p, nil, 0o600); err != nil {
 			return nil, err