fix: restic fails to detect summary event for very short backups

garethgeorge · Nov 28, 2023 · 46b2a85 · 46b2a85
1 parent b385c01
commit 46b2a85
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 15 deletions.
diff --git a/pkg/restic/outputs.go b/pkg/restic/outputs.go
@@ -92,6 +92,8 @@ func readBackupProgressEntries(cmd *exec.Cmd, output io.Reader, callback func(ev
 	scanner := bufio.NewScanner(output)
 	scanner.Split(bufio.ScanLines)
 
+	var summary *BackupProgressEntry
+
 	// first event is handled specially to detect non-JSON output and fast-path out.
 	if scanner.Scan() {
 		var event BackupProgressEntry
@@ -107,13 +109,17 @@ func readBackupProgressEntries(cmd *exec.Cmd, output io.Reader, callback func(ev
 		if err := event.Validate(); err != nil {
 			return nil, err
 		}
+		if callback != nil {
+			callback(&event)
+		}
+		if event.MessageType == "summary" {
+			summary = &event
+		}
 	}
 
 	// remaining events are parsed as JSON
-	var summary *BackupProgressEntry
-
 	for scanner.Scan() {
-		var event *BackupProgressEntry
+		var event BackupProgressEntry
 		if err := json.Unmarshal(scanner.Bytes(), &event); err != nil {
 			return nil, fmt.Errorf("failed to parse JSON: %w", err)
 		}
@@ -122,18 +128,21 @@ func readBackupProgressEntries(cmd *exec.Cmd, output io.Reader, callback func(ev
 		}
 
 		if callback != nil {
-			callback(event)
+			callback(&event)
 		}
-
 		if event.MessageType == "summary" {
-			summary = event
+			summary = &event
 		}
 	}
 
 	if err := scanner.Err(); err != nil {
 		return summary, fmt.Errorf("scanner encountered error: %w", err)
 	}
 
+	if summary == nil {
+		return nil, fmt.Errorf("no summary event found")
+	}
+
 	return summary, nil
 }
 

diff --git a/pkg/restic/restic.go b/pkg/restic/restic.go
@@ -13,7 +13,6 @@ import (
 	"sync"
 
 	v1 "github.com/garethgeorge/resticui/gen/go/v1"
-	"github.com/hashicorp/go-multierror"
 )
 
 type Repo struct {
@@ -99,13 +98,16 @@ func (r *Repo) Backup(ctx context.Context, progressCallback func(*BackupProgress
 	args = append(args, r.extraArgs...)
 	args = append(args, opt.paths...)
 	args = append(args, opt.extraArgs...)
-
+
+	output := bytes.NewBuffer(nil)
 	reader, writer := io.Pipe()
 
+	capture := io.MultiWriter(output, writer)
+
 	cmd := exec.CommandContext(ctx, r.cmd, args...)
 	cmd.Env = append(cmd.Env, r.buildEnv()...)
-	cmd.Stderr = writer
-	cmd.Stdout = writer
+	cmd.Stderr = capture
+	cmd.Stdout = capture
 
 	if err := cmd.Start(); err != nil {
 		return nil, NewCmdError(cmd, nil, err)
@@ -128,20 +130,20 @@ func (r *Repo) Backup(ctx context.Context, progressCallback func(*BackupProgress
 
 	wg.Add(1)
 	go func() {
+		defer capture.Write([]byte("\n"))
 		defer writer.Close()
 		defer wg.Done()
 		if err := cmd.Wait(); err != nil {
-			cmdErr = NewCmdError(cmd, nil, err)
+			cmdErr = err
 		}
 	}()
 
 	wg.Wait()
 
-	var err error
 	if cmdErr != nil || readErr != nil {
-		err = multierror.Append(nil, cmdErr, readErr)
+		return nil, NewCmdError(cmd, output.Bytes(), errors.Join(cmdErr, readErr))
 	}
-	return summary, err
+	return summary, nil
 }
 
 func (r *Repo) Snapshots(ctx context.Context, opts ...GenericOption) ([]*Snapshot, error) {

diff --git a/pkg/restic/restic_test.go b/pkg/restic/restic_test.go
@@ -102,6 +102,35 @@ func TestResticBackup(t *testing.T) {
 	}
 }
 
+func TestResticBackupLots(t *testing.T) {
+	t.Parallel()
+	t.Skip("this test takes a long time to run")
+
+	repo := t.TempDir()
+
+	// create a new repo with cache disabled for testing
+	r := NewRepo(helpers.ResticBinary(t), &v1.Repo{
+		Id:       "test",
+		Uri:      repo,
+		Password: "test",
+	}, WithFlags("--no-cache"))
+	if err := r.Init(context.Background()); err != nil {
+		t.Fatalf("failed to init repo: %v", err)
+	}
+
+	testData := helpers.CreateTestData(t)
+
+	// backup 25 times
+	for i := 0; i < 25; i++ {
+		_, err := r.Backup(context.Background(), func(e *BackupProgressEntry) {
+			t.Logf("backup event: %+v", e)
+		}, WithBackupPaths(testData))
+		if err != nil {
+			t.Fatalf("failed to backup and create new snapshot: %v", err)
+		}
+	}
+}
+
 func TestSnapshot(t *testing.T) {
 	t.Parallel()
 
@@ -215,7 +244,6 @@ func TestResticForget(t *testing.T) {
 		if err != nil {
 			t.Fatalf("failed to backup and create new snapshot: %v", err)
 		}
-
 		ids = append(ids, output.SnapshotId)
 	}