Fail check, restore and backup jobs when they encounter errors

check.go

@@ -69,5 +69,10 @@ func check(ctx *kcontext.KContext, input *ExecPayload) (*Report, error) {
 	}
 
 	ccr.Took = time.Since(start)
-	return &Report{Type: input.Op, Check: &ccr}, nil
+
+	report := &Report{Type: input.Op, Check: &ccr}
+	if ccr.Errors != 0 {
+		return report, fmt.Errorf("check failed: %d of %d snapshot(s) had errors", ccr.Errors, len(snapshotIDs))
+	}
+	return report, nil
 }

plaklet.go

@@ -170,15 +170,18 @@ func Main(args []string) int {
 	ctx.Events().Close()
 	listener.Wait()
 
+	// listener.Wait() has drained every event, so the final State is complete
+	// and race-free here — safe to fold event-only errors into the verdict.
+	if err == nil {
+		err = terminalError(input.Op, report, listener.State())
+	}
+
 	if err != nil {
 		send(&ExecReply{Type: ReplyFailure, Message: fmt.Sprintf("%s failed: %s", input.Op, err)})
 		return 0
 	}
 
 	if report != nil {
-		if w := backupWarning(input.Op, report); w != nil {
-			send(w)
-		}
 		if raw, merrr := json.Marshal(report); merrr == nil {
 			send(&ExecReply{Type: ReplyReport, Report: raw})
 		}
@@ -187,21 +190,35 @@ func Main(args []string) int {
 	return 0
 }
 
-// backupWarning returns a warning reply when a backup committed a snapshot but
-// couldn't read some files, or nil otherwise. Such a run is a
-// success-with-warnings, not a failure (standard plakar semantics: the snapshot
-// is still valid). The per-file error count lives in the report but nothing
-// downstream inspects it, so we surface it as a warning the operator sees while
-// the job still succeeds.
-func backupWarning(op string, report *Report) *ExecReply {
-	if report == nil || report.Backup == nil || report.Backup.Errors <= 0 {
+// terminalError decides whether a completed operation should fail the job, for
+// the operations whose failures surface only after the fact rather than as a
+// returned error. A backup or restore commits/returns nil even when it couldn't
+// read or write some entries; check tallies snapshot errors in its own report
+// and already returns an error itself. We treat any such error count as a job
+// failure — a partial run is not a clean success. state carries the per-entry
+// error counters folded from the (now fully drained) event bus, used for
+// restore whose failures live only in events. Mutates report to record the
+// count. Returns nil when the run is clean.
+func terminalError(op string, report *Report, state State) error {
+	if report == nil {
 		return nil
 	}
-	return &ExecReply{
-		Type: ReplyWarning,
-		Message: fmt.Sprintf("%s completed with %d file error(s); those files were skipped",
-			op, report.Backup.Errors),
+	switch op {
+	case "backup":
+		if report.Backup != nil && report.Backup.Errors > 0 {
+			return fmt.Errorf("backup failed: %d file(s) could not be read", report.Backup.Errors)
+		}
+	case "restore":
+		if report.Restore != nil {
+			entryErrors := state.Paths.Error + state.Files.Error + state.Dirs.Error +
+				state.Symlinks.Error + state.Xattrs.Error
+			if entryErrors != 0 {
+				report.Restore.Errors = entryErrors
+				return fmt.Errorf("restore failed: %d entries could not be restored", entryErrors)
+			}
+		}
 	}
+	return nil
 }
 
 // dispatch routes an operation to its handler. Only the operations a remote edge

report.go

@@ -59,6 +59,7 @@ type RestoreReport struct {
 	Took        time.Duration `json:"took"`
 	LogicalSize uint64        `json:"logical_size"`
 	Content     BackupContent `json:"content"`
+	Errors      uint64        `json:"errors"`
 	Store       StoreIO       `json:"store"`
 }
 

terminal_error_test.go

@@ -0,0 +1,39 @@
+package plaklet
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestTerminalError(t *testing.T) {
+	// A clean backup/restore is not a failure.
+	require.NoError(t, terminalError("backup", &Report{Backup: &BackupReport{Errors: 0}}, State{}))
+	require.NoError(t, terminalError("restore", &Report{Restore: &RestoreReport{}}, State{}))
+
+	// Nil report / nil section are safe no-ops.
+	require.NoError(t, terminalError("backup", nil, State{}))
+	require.NoError(t, terminalError("backup", &Report{}, State{}))
+	require.NoError(t, terminalError("restore", &Report{}, State{}))
+
+	// A backup that couldn't read some files fails the job (same semantics as
+	// restore and check — a partial run is not a clean success).
+	err := terminalError("backup", &Report{Backup: &BackupReport{Errors: 3}}, State{})
+	require.Error(t, err)
+	require.True(t, strings.Contains(err.Error(), "3"), "message should carry the error count: %q", err)
+
+	// A restore whose entries failed to export fails the job; the per-entry
+	// counts come from the drained event State, and are folded into the report.
+	var st State
+	st.Files.Error = 2
+	st.Symlinks.Error = 1
+	rep := &Report{Restore: &RestoreReport{}}
+	err = terminalError("restore", rep, st)
+	require.Error(t, err)
+	require.Equal(t, uint64(3), rep.Restore.Errors)
+	require.True(t, strings.Contains(err.Error(), "3"), "message should carry the error count: %q", err)
+
+	// Operations without a post-hoc failure notion (e.g. sync) are unaffected.
+	require.NoError(t, terminalError("sync", &Report{}, State{}))
+}