From 91e0fdaffecedbf39aea776c3448eb27586aaaa7 Mon Sep 17 00:00:00 2001
From: garethgeorge <garethgeorge97@gmail.com>
Date: Thu, 13 Jun 2024 17:18:05 -0700
Subject: [PATCH] fix: make backup and restore operations more robust to
 non-JSON output events

---
 pkg/restic/error.go   | 10 ++++----
 pkg/restic/outputs.go | 55 ++++++++++---------------------------------
 2 files changed, 17 insertions(+), 48 deletions(-)

diff --git a/pkg/restic/error.go b/pkg/restic/error.go
index 410af4bb..bb1da609 100644
--- a/pkg/restic/error.go
+++ b/pkg/restic/error.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"os/exec"
-	"strings"
 )
 
 const outputBufferLimit = 1000
@@ -61,13 +60,12 @@ func (e *ErrorWithOutput) Is(target error) bool {
 
 // newErrorWithOutput creates a new error with the given output.
 func newErrorWithOutput(err error, output string) error {
-	firstNewLine := strings.Index(output, "\n")
-	if firstNewLine > 0 {
-		output = output[:firstNewLine]
+	if output == "" {
+		return err
 	}
 
-	if len(output) == 0 {
-		return err
+	if len(output) > outputBufferLimit {
+		output = output[:outputBufferLimit] + fmt.Sprintf("\n... %d bytes truncated ...\n", len(output)-outputBufferLimit)
 	}
 
 	return &ErrorWithOutput{
diff --git a/pkg/restic/outputs.go b/pkg/restic/outputs.go
index 1944cf3a..8a4a9564 100644
--- a/pkg/restic/outputs.go
+++ b/pkg/restic/outputs.go
@@ -2,6 +2,7 @@ package restic
 
 import (
 	"bufio"
+	"bytes"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -96,34 +97,19 @@ func readBackupProgressEntries(output io.Reader, callback func(event *BackupProg
 	scanner := bufio.NewScanner(output)
 	scanner.Split(bufio.ScanLines)
 
-	var summary *BackupProgressEntry
+	nonJSONOutput := bytes.NewBuffer(nil)
 
-	// first event is handled specially to detect non-JSON output and fast-path out.
-	if scanner.Scan() {
-		var event BackupProgressEntry
-		if err := json.Unmarshal(scanner.Bytes(), &event); err != nil {
-			return nil, fmt.Errorf("command output was not JSON: %w", err)
-		}
-		if err := event.Validate(); err != nil {
-			return nil, err
-		}
-		if callback != nil {
-			callback(&event)
-		}
-		if event.MessageType == "summary" {
-			summary = &event
-		}
-	}
+	var summary *BackupProgressEntry
 
 	// remaining events are parsed as JSON
 	for scanner.Scan() {
 		var event BackupProgressEntry
 		if err := json.Unmarshal(scanner.Bytes(), &event); err != nil {
-			// skip it. This is a best-effort attempt to parse the output.
+			nonJSONOutput.Write(scanner.Bytes())
 			continue
 		}
 		if err := event.Validate(); err != nil {
-			// skip it. This is a best-effort attempt to parse the output.
+			nonJSONOutput.Write(scanner.Bytes())
 			continue
 		}
 		if callback != nil {
@@ -134,10 +120,10 @@ func readBackupProgressEntries(output io.Reader, callback func(event *BackupProg
 		}
 	}
 	if err := scanner.Err(); err != nil {
-		return summary, fmt.Errorf("scanner encountered error: %w", err)
+		return summary, newErrorWithOutput(err, nonJSONOutput.String())
 	}
 	if summary == nil {
-		return nil, fmt.Errorf("no summary event found")
+		return nil, newErrorWithOutput(errors.New("no summary event found"), nonJSONOutput.String())
 	}
 	return summary, nil
 }
@@ -235,35 +221,20 @@ func readRestoreProgressEntries(output io.Reader, callback func(event *RestorePr
 	scanner := bufio.NewScanner(output)
 	scanner.Split(bufio.ScanLines)
 
-	var summary *RestoreProgressEntry
-
-	// first event is handled specially to detect non-JSON output and fast-path out.
-	if scanner.Scan() {
-		var event RestoreProgressEntry
+	nonJSONOutput := bytes.NewBuffer(nil)
 
-		if err := json.Unmarshal(scanner.Bytes(), &event); err != nil {
-			return nil, fmt.Errorf("command output was not JSON: %w", err)
-		}
-		if err := event.Validate(); err != nil {
-			return nil, err
-		}
-		if callback != nil {
-			callback(&event)
-		}
-		if event.MessageType == "summary" {
-			summary = &event
-		}
-	}
+	var summary *RestoreProgressEntry
 
 	// remaining events are parsed as JSON
 	for scanner.Scan() {
 		var event RestoreProgressEntry
 		if err := json.Unmarshal(scanner.Bytes(), &event); err != nil {
-			// skip it. Best effort parsing, restic will return with a non-zero exit code if it fails.
+			nonJSONOutput.Write(scanner.Bytes())
 			continue
 		}
 		if err := event.Validate(); err != nil {
 			// skip it. Best effort parsing, restic will return with a non-zero exit code if it fails.
+			nonJSONOutput.Write(scanner.Bytes())
 			continue
 		}
 
@@ -276,11 +247,11 @@ func readRestoreProgressEntries(output io.Reader, callback func(event *RestorePr
 	}
 
 	if err := scanner.Err(); err != nil {
-		return summary, fmt.Errorf("scanner encountered error: %w", err)
+		return summary, newErrorWithOutput(err, nonJSONOutput.String())
 	}
 
 	if summary == nil {
-		return nil, fmt.Errorf("no summary event found")
+		return nil, newErrorWithOutput(errors.New("no summary event found"), nonJSONOutput.String())
 	}
 
 	return summary, nil