From f4bdfea1c9a9ff2e5e9d2bc48eda44afc58cf1b0 Mon Sep 17 00:00:00 2001 From: Michael Terry Date: Sat, 27 Jul 2024 19:00:43 -0400 Subject: [PATCH 1/4] backup: print scanner errors to stderr, not stdout --- internal/ui/backup/text.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/ui/backup/text.go b/internal/ui/backup/text.go index f96746739..4e3f1aec4 100644 --- a/internal/ui/backup/text.go +++ b/internal/ui/backup/text.go @@ -15,7 +15,8 @@ import ( type TextProgress struct { *ui.Message - term *termstatus.Terminal + term *termstatus.Terminal + verbosity uint } // assert that Backup implements the ProgressPrinter interface @@ -24,8 +25,9 @@ var _ ProgressPrinter = &TextProgress{} // NewTextProgress returns a new backup progress reporter. func NewTextProgress(term *termstatus.Terminal, verbosity uint) *TextProgress { return &TextProgress{ - Message: ui.NewMessage(term, verbosity), - term: term, + Message: ui.NewMessage(term, verbosity), + term: term, + verbosity: verbosity, } } @@ -73,7 +75,9 @@ func (b *TextProgress) Update(total, processed Counter, errors uint, currentFile // ScannerError is the error callback function for the scanner, it prints the // error in verbose mode and returns nil. func (b *TextProgress) ScannerError(_ string, err error) error { - b.V("scan: %v\n", err) + if b.verbosity >= 2 { + b.E("scan: %v\n", err) + } return nil } From ad2585af679a6c5a95056ede9555909588d24ebb Mon Sep 17 00:00:00 2001 From: Michael Terry Date: Sat, 27 Jul 2024 19:04:34 -0400 Subject: [PATCH 2/4] backup: show actual error strings in --json mode Previously, an error JSON fragment would look like: {"message_type": "error", "error": {}} This is because encoding/json cannot marshal an error interface. Instead, we now call .Error() to get the string value. --- internal/ui/backup/json.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/ui/backup/json.go b/internal/ui/backup/json.go index 64b5de13b..81bbd1047 100644 --- a/internal/ui/backup/json.go +++ b/internal/ui/backup/json.go @@ -68,7 +68,7 @@ func (b *JSONProgress) Update(total, processed Counter, errors uint, currentFile func (b *JSONProgress) ScannerError(item string, err error) error { b.error(errorUpdate{ MessageType: "error", - Error: err, + Error: err.Error(), During: "scan", Item: item, }) @@ -79,7 +79,7 @@ func (b *JSONProgress) ScannerError(item string, err error) error { func (b *JSONProgress) Error(item string, err error) error { b.error(errorUpdate{ MessageType: "error", - Error: err, + Error: err.Error(), During: "archival", Item: item, }) @@ -208,7 +208,7 @@ type statusUpdate struct { type errorUpdate struct { MessageType string `json:"message_type"` // "error" - Error error `json:"error"` + Error string `json:"error"` During string `json:"during"` Item string `json:"item"` } From a376323331574a4f5dd00177d6ac2c8e5f7d760c Mon Sep 17 00:00:00 2001 From: Michael Terry Date: Sat, 27 Jul 2024 19:06:26 -0400 Subject: [PATCH 3/4] restore: print JSON versions of errors in --json mode Previously, they were printed as freeform text. This also adds a ui.Terminal interface to make writing tests easier and also adds a few tests. --- changelog/unreleased/issue-4944 | 8 ++++ changelog/unreleased/issue-4945 | 8 ++++ cmd/restic/cmd_restore.go | 3 +- internal/restorer/restorer_test.go | 3 ++ internal/ui/backup/json.go | 5 +-- internal/ui/backup/json_test.go | 27 ++++++++++++++ internal/ui/backup/text.go | 4 +- internal/ui/backup/text_test.go | 27 ++++++++++++++ internal/ui/message.go | 6 +-- internal/ui/mock.go | 22 +++++++++++ internal/ui/restore/json.go | 25 ++++++++++++- internal/ui/restore/json_test.go | 24 ++++++++---- internal/ui/restore/progress.go | 17 ++++++--- internal/ui/restore/progress_test.go | 55 +++++++++++++++++++++------- internal/ui/restore/text.go | 24 ++++++------ internal/ui/restore/text_test.go | 36 ++++++++---------- internal/ui/terminal.go | 10 +++++ 17 files changed, 234 insertions(+), 70 deletions(-) create mode 100644 changelog/unreleased/issue-4944 create mode 100644 changelog/unreleased/issue-4945 create mode 100644 internal/ui/backup/json_test.go create mode 100644 internal/ui/backup/text_test.go create mode 100644 internal/ui/mock.go create mode 100644 internal/ui/terminal.go diff --git a/changelog/unreleased/issue-4944 b/changelog/unreleased/issue-4944 new file mode 100644 index 000000000..02f5ae341 --- /dev/null +++ b/changelog/unreleased/issue-4944 @@ -0,0 +1,8 @@ +Enhancement: Print JSON-formatted errors during `restore --json` + +Restic printed any restore errors directly to the console as freeform +text messages, even with `--json`. Restic now prints them as JSON formatted +messages when `--json` is passed. + +https://github.com/restic/restic/issues/4944 +https://github.com/restic/restic/pull/4946 diff --git a/changelog/unreleased/issue-4945 b/changelog/unreleased/issue-4945 new file mode 100644 index 000000000..7bbf69fac --- /dev/null +++ b/changelog/unreleased/issue-4945 @@ -0,0 +1,8 @@ +Bugfix: Include missing backup error text with `--json` + +Restic was not actually providing the text of an error message during +backup if `--json` was passed, instead only printing `"error": {}`. +Restic now includes the error text in JSON output. + +https://github.com/restic/restic/issues/4945 +https://github.com/restic/restic/pull/4946 diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index 89942f4cf..eda608802 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -164,9 +164,8 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, totalErrors := 0 res.Error = func(location string, err error) error { - msg.E("ignoring error for %s: %s\n", location, err) totalErrors++ - return nil + return progress.Error(location, err) } res.Warn = func(message string) { msg.E("Warning: %s\n", message) diff --git a/internal/restorer/restorer_test.go b/internal/restorer/restorer_test.go index 191f3b8ef..a6de50556 100644 --- a/internal/restorer/restorer_test.go +++ b/internal/restorer/restorer_test.go @@ -989,6 +989,9 @@ type printerMock struct { func (p *printerMock) Update(_ restoreui.State, _ time.Duration) { } +func (p *printerMock) Error(item string, err error) error { + return nil +} func (p *printerMock) CompleteItem(action restoreui.ItemAction, item string, size uint64) { } func (p *printerMock) Finish(s restoreui.State, _ time.Duration) { diff --git a/internal/ui/backup/json.go b/internal/ui/backup/json.go index 81bbd1047..bb6685136 100644 --- a/internal/ui/backup/json.go +++ b/internal/ui/backup/json.go @@ -7,14 +7,13 @@ import ( "github.com/restic/restic/internal/archiver" "github.com/restic/restic/internal/restic" "github.com/restic/restic/internal/ui" - "github.com/restic/restic/internal/ui/termstatus" ) // JSONProgress reports progress for the `backup` command in JSON. type JSONProgress struct { *ui.Message - term *termstatus.Terminal + term ui.Terminal v uint } @@ -22,7 +21,7 @@ type JSONProgress struct { var _ ProgressPrinter = &JSONProgress{} // NewJSONProgress returns a new backup progress reporter. -func NewJSONProgress(term *termstatus.Terminal, verbosity uint) *JSONProgress { +func NewJSONProgress(term ui.Terminal, verbosity uint) *JSONProgress { return &JSONProgress{ Message: ui.NewMessage(term, verbosity), term: term, diff --git a/internal/ui/backup/json_test.go b/internal/ui/backup/json_test.go new file mode 100644 index 000000000..4846279b3 --- /dev/null +++ b/internal/ui/backup/json_test.go @@ -0,0 +1,27 @@ +package backup + +import ( + "testing" + + "github.com/restic/restic/internal/errors" + "github.com/restic/restic/internal/test" + "github.com/restic/restic/internal/ui" +) + +func createJSONProgress() (*ui.MockTerminal, ProgressPrinter) { + term := &ui.MockTerminal{} + printer := NewJSONProgress(term, 3) + return term, printer +} + +func TestJSONError(t *testing.T) { + term, printer := createJSONProgress() + test.Equals(t, printer.Error("/path", errors.New("error \"message\"")), nil) + test.Equals(t, []string{"{\"message_type\":\"error\",\"error\":\"error \\\"message\\\"\",\"during\":\"archival\",\"item\":\"/path\"}\n"}, term.Errors) +} + +func TestJSONScannerError(t *testing.T) { + term, printer := createJSONProgress() + test.Equals(t, printer.ScannerError("/path", errors.New("error \"message\"")), nil) + test.Equals(t, []string{"{\"message_type\":\"error\",\"error\":\"error \\\"message\\\"\",\"during\":\"scan\",\"item\":\"/path\"}\n"}, term.Errors) +} diff --git a/internal/ui/backup/text.go b/internal/ui/backup/text.go index 4e3f1aec4..097f0d0d8 100644 --- a/internal/ui/backup/text.go +++ b/internal/ui/backup/text.go @@ -15,7 +15,7 @@ import ( type TextProgress struct { *ui.Message - term *termstatus.Terminal + term ui.Terminal verbosity uint } @@ -23,7 +23,7 @@ type TextProgress struct { var _ ProgressPrinter = &TextProgress{} // NewTextProgress returns a new backup progress reporter. -func NewTextProgress(term *termstatus.Terminal, verbosity uint) *TextProgress { +func NewTextProgress(term ui.Terminal, verbosity uint) *TextProgress { return &TextProgress{ Message: ui.NewMessage(term, verbosity), term: term, diff --git a/internal/ui/backup/text_test.go b/internal/ui/backup/text_test.go new file mode 100644 index 000000000..39338a50c --- /dev/null +++ b/internal/ui/backup/text_test.go @@ -0,0 +1,27 @@ +package backup + +import ( + "testing" + + "github.com/restic/restic/internal/errors" + "github.com/restic/restic/internal/test" + "github.com/restic/restic/internal/ui" +) + +func createTextProgress() (*ui.MockTerminal, ProgressPrinter) { + term := &ui.MockTerminal{} + printer := NewTextProgress(term, 3) + return term, printer +} + +func TestError(t *testing.T) { + term, printer := createTextProgress() + test.Equals(t, printer.Error("/path", errors.New("error \"message\"")), nil) + test.Equals(t, []string{"error: error \"message\"\n"}, term.Errors) +} + +func TestScannerError(t *testing.T) { + term, printer := createTextProgress() + test.Equals(t, printer.ScannerError("/path", errors.New("error \"message\"")), nil) + test.Equals(t, []string{"scan: error \"message\"\n"}, term.Errors) +} diff --git a/internal/ui/message.go b/internal/ui/message.go index 38cdaf301..6ad5a439e 100644 --- a/internal/ui/message.go +++ b/internal/ui/message.go @@ -2,19 +2,17 @@ package ui import ( "fmt" - - "github.com/restic/restic/internal/ui/termstatus" ) // Message reports progress with messages of different verbosity. type Message struct { - term *termstatus.Terminal + term Terminal v uint } // NewMessage returns a message progress reporter with underlying terminal // term. -func NewMessage(term *termstatus.Terminal, verbosity uint) *Message { +func NewMessage(term Terminal, verbosity uint) *Message { return &Message{ term: term, v: verbosity, diff --git a/internal/ui/mock.go b/internal/ui/mock.go new file mode 100644 index 000000000..5a4debb02 --- /dev/null +++ b/internal/ui/mock.go @@ -0,0 +1,22 @@ +package ui + +type MockTerminal struct { + Output []string + Errors []string +} + +func (m *MockTerminal) Print(line string) { + m.Output = append(m.Output, line) +} + +func (m *MockTerminal) Error(line string) { + m.Errors = append(m.Errors, line) +} + +func (m *MockTerminal) SetStatus(lines []string) { + m.Output = append([]string{}, lines...) +} + +func (m *MockTerminal) CanUpdateStatus() bool { + return true +} diff --git a/internal/ui/restore/json.go b/internal/ui/restore/json.go index c248a7951..4135dd667 100644 --- a/internal/ui/restore/json.go +++ b/internal/ui/restore/json.go @@ -7,11 +7,11 @@ import ( ) type jsonPrinter struct { - terminal term + terminal ui.Terminal verbosity uint } -func NewJSONProgress(terminal term, verbosity uint) ProgressPrinter { +func NewJSONProgress(terminal ui.Terminal, verbosity uint) ProgressPrinter { return &jsonPrinter{ terminal: terminal, verbosity: verbosity, @@ -22,6 +22,10 @@ func (t *jsonPrinter) print(status interface{}) { t.terminal.Print(ui.ToJSONString(status)) } +func (t *jsonPrinter) error(status interface{}) { + t.terminal.Error(ui.ToJSONString(status)) +} + func (t *jsonPrinter) Update(p State, duration time.Duration) { status := statusUpdate{ MessageType: "status", @@ -41,6 +45,16 @@ func (t *jsonPrinter) Update(p State, duration time.Duration) { t.print(status) } +func (t *jsonPrinter) Error(item string, err error) error { + t.error(errorUpdate{ + MessageType: "error", + Error: err.Error(), + During: "restore", + Item: item, + }) + return nil +} + func (t *jsonPrinter) CompleteItem(messageType ItemAction, item string, size uint64) { if t.verbosity < 3 { return @@ -99,6 +113,13 @@ type statusUpdate struct { BytesSkipped uint64 `json:"bytes_skipped,omitempty"` } +type errorUpdate struct { + MessageType string `json:"message_type"` // "error" + Error string `json:"error"` + During string `json:"during"` + Item string `json:"item"` +} + type verboseUpdate struct { MessageType string `json:"message_type"` // "verbose_status" Action string `json:"action"` diff --git a/internal/ui/restore/json_test.go b/internal/ui/restore/json_test.go index 06a70d5dc..1e0f80a4f 100644 --- a/internal/ui/restore/json_test.go +++ b/internal/ui/restore/json_test.go @@ -4,11 +4,13 @@ import ( "testing" "time" + "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/test" + "github.com/restic/restic/internal/ui" ) -func createJSONProgress() (*mockTerm, ProgressPrinter) { - term := &mockTerm{} +func createJSONProgress() (*ui.MockTerminal, ProgressPrinter) { + term := &ui.MockTerminal{} printer := NewJSONProgress(term, 3) return term, printer } @@ -16,31 +18,31 @@ func createJSONProgress() (*mockTerm, ProgressPrinter) { func TestJSONPrintUpdate(t *testing.T) { term, printer := createJSONProgress() printer.Update(State{3, 11, 0, 29, 47, 0}, 5*time.Second) - test.Equals(t, []string{"{\"message_type\":\"status\",\"seconds_elapsed\":5,\"percent_done\":0.6170212765957447,\"total_files\":11,\"files_restored\":3,\"total_bytes\":47,\"bytes_restored\":29}\n"}, term.output) + test.Equals(t, []string{"{\"message_type\":\"status\",\"seconds_elapsed\":5,\"percent_done\":0.6170212765957447,\"total_files\":11,\"files_restored\":3,\"total_bytes\":47,\"bytes_restored\":29}\n"}, term.Output) } func TestJSONPrintUpdateWithSkipped(t *testing.T) { term, printer := createJSONProgress() printer.Update(State{3, 11, 2, 29, 47, 59}, 5*time.Second) - test.Equals(t, []string{"{\"message_type\":\"status\",\"seconds_elapsed\":5,\"percent_done\":0.6170212765957447,\"total_files\":11,\"files_restored\":3,\"files_skipped\":2,\"total_bytes\":47,\"bytes_restored\":29,\"bytes_skipped\":59}\n"}, term.output) + test.Equals(t, []string{"{\"message_type\":\"status\",\"seconds_elapsed\":5,\"percent_done\":0.6170212765957447,\"total_files\":11,\"files_restored\":3,\"files_skipped\":2,\"total_bytes\":47,\"bytes_restored\":29,\"bytes_skipped\":59}\n"}, term.Output) } func TestJSONPrintSummaryOnSuccess(t *testing.T) { term, printer := createJSONProgress() printer.Finish(State{11, 11, 0, 47, 47, 0}, 5*time.Second) - test.Equals(t, []string{"{\"message_type\":\"summary\",\"seconds_elapsed\":5,\"total_files\":11,\"files_restored\":11,\"total_bytes\":47,\"bytes_restored\":47}\n"}, term.output) + test.Equals(t, []string{"{\"message_type\":\"summary\",\"seconds_elapsed\":5,\"total_files\":11,\"files_restored\":11,\"total_bytes\":47,\"bytes_restored\":47}\n"}, term.Output) } func TestJSONPrintSummaryOnErrors(t *testing.T) { term, printer := createJSONProgress() printer.Finish(State{3, 11, 0, 29, 47, 0}, 5*time.Second) - test.Equals(t, []string{"{\"message_type\":\"summary\",\"seconds_elapsed\":5,\"total_files\":11,\"files_restored\":3,\"total_bytes\":47,\"bytes_restored\":29}\n"}, term.output) + test.Equals(t, []string{"{\"message_type\":\"summary\",\"seconds_elapsed\":5,\"total_files\":11,\"files_restored\":3,\"total_bytes\":47,\"bytes_restored\":29}\n"}, term.Output) } func TestJSONPrintSummaryOnSuccessWithSkipped(t *testing.T) { term, printer := createJSONProgress() printer.Finish(State{11, 11, 2, 47, 47, 59}, 5*time.Second) - test.Equals(t, []string{"{\"message_type\":\"summary\",\"seconds_elapsed\":5,\"total_files\":11,\"files_restored\":11,\"files_skipped\":2,\"total_bytes\":47,\"bytes_restored\":47,\"bytes_skipped\":59}\n"}, term.output) + test.Equals(t, []string{"{\"message_type\":\"summary\",\"seconds_elapsed\":5,\"total_files\":11,\"files_restored\":11,\"files_skipped\":2,\"total_bytes\":47,\"bytes_restored\":47,\"bytes_skipped\":59}\n"}, term.Output) } func TestJSONPrintCompleteItem(t *testing.T) { @@ -57,6 +59,12 @@ func TestJSONPrintCompleteItem(t *testing.T) { } { term, printer := createJSONProgress() printer.CompleteItem(data.action, "test", data.size) - test.Equals(t, []string{data.expected}, term.output) + test.Equals(t, []string{data.expected}, term.Output) } } + +func TestJSONError(t *testing.T) { + term, printer := createJSONProgress() + test.Equals(t, printer.Error("/path", errors.New("error \"message\"")), nil) + test.Equals(t, []string{"{\"message_type\":\"error\",\"error\":\"error \\\"message\\\"\",\"during\":\"restore\",\"item\":\"/path\"}\n"}, term.Errors) +} diff --git a/internal/ui/restore/progress.go b/internal/ui/restore/progress.go index 67b15f07e..06f4c86aa 100644 --- a/internal/ui/restore/progress.go +++ b/internal/ui/restore/progress.go @@ -32,13 +32,9 @@ type progressInfoEntry struct { bytesTotal uint64 } -type term interface { - Print(line string) - SetStatus(lines []string) -} - type ProgressPrinter interface { Update(progress State, duration time.Duration) + Error(item string, err error) error CompleteItem(action ItemAction, item string, size uint64) Finish(progress State, duration time.Duration) } @@ -139,6 +135,17 @@ func (p *Progress) ReportDeletedFile(name string) { p.printer.CompleteItem(ActionDeleted, name, 0) } +func (p *Progress) Error(item string, err error) error { + if p == nil { + return nil + } + + p.m.Lock() + defer p.m.Unlock() + + return p.printer.Error(item, err) +} + func (p *Progress) Finish() { p.updater.Done() } diff --git a/internal/ui/restore/progress_test.go b/internal/ui/restore/progress_test.go index 4a6304741..b01440bee 100644 --- a/internal/ui/restore/progress_test.go +++ b/internal/ui/restore/progress_test.go @@ -4,6 +4,7 @@ import ( "testing" "time" + "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/test" ) @@ -23,9 +24,18 @@ type itemTraceEntry struct { } type itemTrace []itemTraceEntry + +type errorTraceEntry struct { + item string + err error +} + +type errorTrace []errorTraceEntry + type mockPrinter struct { - trace printerTrace - items itemTrace + trace printerTrace + items itemTrace + errors errorTrace } const mockFinishDuration = 42 * time.Second @@ -33,6 +43,10 @@ const mockFinishDuration = 42 * time.Second func (p *mockPrinter) Update(progress State, duration time.Duration) { p.trace = append(p.trace, printerTraceEntry{progress, duration, false}) } +func (p *mockPrinter) Error(item string, err error) error { + p.errors = append(p.errors, errorTraceEntry{item, err}) + return nil +} func (p *mockPrinter) CompleteItem(action ItemAction, item string, size uint64) { p.items = append(p.items, itemTraceEntry{action, item, size}) } @@ -40,20 +54,21 @@ func (p *mockPrinter) Finish(progress State, _ time.Duration) { p.trace = append(p.trace, printerTraceEntry{progress, mockFinishDuration, true}) } -func testProgress(fn func(progress *Progress) bool) (printerTrace, itemTrace) { +func testProgress(fn func(progress *Progress) bool) (printerTrace, itemTrace, errorTrace) { printer := &mockPrinter{} progress := NewProgress(printer, 0) final := fn(progress) progress.update(0, final) trace := append(printerTrace{}, printer.trace...) items := append(itemTrace{}, printer.items...) + errors := append(errorTrace{}, printer.errors...) // cleanup to avoid goroutine leak, but copy trace first progress.Finish() - return trace, items + return trace, items, errors } func TestNew(t *testing.T) { - result, items := testProgress(func(progress *Progress) bool { + result, items, _ := testProgress(func(progress *Progress) bool { return false }) test.Equals(t, printerTrace{ @@ -65,7 +80,7 @@ func TestNew(t *testing.T) { func TestAddFile(t *testing.T) { fileSize := uint64(100) - result, items := testProgress(func(progress *Progress) bool { + result, items, _ := testProgress(func(progress *Progress) bool { progress.AddFile(fileSize) return false }) @@ -79,7 +94,7 @@ func TestFirstProgressOnAFile(t *testing.T) { expectedBytesWritten := uint64(5) expectedBytesTotal := uint64(100) - result, items := testProgress(func(progress *Progress) bool { + result, items, _ := testProgress(func(progress *Progress) bool { progress.AddFile(expectedBytesTotal) progress.AddProgress("test", ActionFileUpdated, expectedBytesWritten, expectedBytesTotal) return false @@ -93,7 +108,7 @@ func TestFirstProgressOnAFile(t *testing.T) { func TestLastProgressOnAFile(t *testing.T) { fileSize := uint64(100) - result, items := testProgress(func(progress *Progress) bool { + result, items, _ := testProgress(func(progress *Progress) bool { progress.AddFile(fileSize) progress.AddProgress("test", ActionFileUpdated, 30, fileSize) progress.AddProgress("test", ActionFileUpdated, 35, fileSize) @@ -111,7 +126,7 @@ func TestLastProgressOnAFile(t *testing.T) { func TestLastProgressOnLastFile(t *testing.T) { fileSize := uint64(100) - result, items := testProgress(func(progress *Progress) bool { + result, items, _ := testProgress(func(progress *Progress) bool { progress.AddFile(fileSize) progress.AddFile(50) progress.AddProgress("test1", ActionFileUpdated, 50, 50) @@ -131,7 +146,7 @@ func TestLastProgressOnLastFile(t *testing.T) { func TestSummaryOnSuccess(t *testing.T) { fileSize := uint64(100) - result, _ := testProgress(func(progress *Progress) bool { + result, _, _ := testProgress(func(progress *Progress) bool { progress.AddFile(fileSize) progress.AddFile(50) progress.AddProgress("test1", ActionFileUpdated, 50, 50) @@ -146,7 +161,7 @@ func TestSummaryOnSuccess(t *testing.T) { func TestSummaryOnErrors(t *testing.T) { fileSize := uint64(100) - result, _ := testProgress(func(progress *Progress) bool { + result, _, _ := testProgress(func(progress *Progress) bool { progress.AddFile(fileSize) progress.AddFile(50) progress.AddProgress("test1", ActionFileUpdated, 50, 50) @@ -161,7 +176,7 @@ func TestSummaryOnErrors(t *testing.T) { func TestSkipFile(t *testing.T) { fileSize := uint64(100) - result, items := testProgress(func(progress *Progress) bool { + result, items, _ := testProgress(func(progress *Progress) bool { progress.AddSkippedFile("test", fileSize) return true }) @@ -176,7 +191,7 @@ func TestSkipFile(t *testing.T) { func TestProgressTypes(t *testing.T) { fileSize := uint64(100) - _, items := testProgress(func(progress *Progress) bool { + _, items, _ := testProgress(func(progress *Progress) bool { progress.AddFile(fileSize) progress.AddFile(0) progress.AddProgress("dir", ActionDirRestored, fileSize, fileSize) @@ -190,3 +205,17 @@ func TestProgressTypes(t *testing.T) { itemTraceEntry{ActionDeleted, "del", 0}, }, items) } + +func TestProgressError(t *testing.T) { + err1 := errors.New("err1") + err2 := errors.New("err2") + _, _, errors := testProgress(func(progress *Progress) bool { + test.Equals(t, progress.Error("first", err1), nil) + test.Equals(t, progress.Error("second", err2), nil) + return true + }) + test.Equals(t, errorTrace{ + errorTraceEntry{"first", err1}, + errorTraceEntry{"second", err2}, + }, errors) +} diff --git a/internal/ui/restore/text.go b/internal/ui/restore/text.go index ec512f369..ba0dcd007 100644 --- a/internal/ui/restore/text.go +++ b/internal/ui/restore/text.go @@ -8,14 +8,15 @@ import ( ) type textPrinter struct { - terminal term - verbosity uint + *ui.Message + + terminal ui.Terminal } -func NewTextProgress(terminal term, verbosity uint) ProgressPrinter { +func NewTextProgress(terminal ui.Terminal, verbosity uint) ProgressPrinter { return &textPrinter{ - terminal: terminal, - verbosity: verbosity, + Message: ui.NewMessage(terminal, verbosity), + terminal: terminal, } } @@ -33,11 +34,12 @@ func (t *textPrinter) Update(p State, duration time.Duration) { t.terminal.SetStatus([]string{progress}) } -func (t *textPrinter) CompleteItem(messageType ItemAction, item string, size uint64) { - if t.verbosity < 3 { - return - } +func (t *textPrinter) Error(item string, err error) error { + t.E("ignoring error for %s: %s\n", item, err) + return nil +} +func (t *textPrinter) CompleteItem(messageType ItemAction, item string, size uint64) { var action string switch messageType { case ActionDirRestored: @@ -57,9 +59,9 @@ func (t *textPrinter) CompleteItem(messageType ItemAction, item string, size uin } if messageType == ActionDirRestored || messageType == ActionOtherRestored || messageType == ActionDeleted { - t.terminal.Print(fmt.Sprintf("%-9v %v", action, item)) + t.VV("%-9v %v", action, item) } else { - t.terminal.Print(fmt.Sprintf("%-9v %v with size %v", action, item, ui.FormatBytes(size))) + t.VV("%-9v %v with size %v", action, item, ui.FormatBytes(size)) } } diff --git a/internal/ui/restore/text_test.go b/internal/ui/restore/text_test.go index b198a27df..4ffb1615d 100644 --- a/internal/ui/restore/text_test.go +++ b/internal/ui/restore/text_test.go @@ -4,23 +4,13 @@ import ( "testing" "time" + "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/test" + "github.com/restic/restic/internal/ui" ) -type mockTerm struct { - output []string -} - -func (m *mockTerm) Print(line string) { - m.output = append(m.output, line) -} - -func (m *mockTerm) SetStatus(lines []string) { - m.output = append([]string{}, lines...) -} - -func createTextProgress() (*mockTerm, ProgressPrinter) { - term := &mockTerm{} +func createTextProgress() (*ui.MockTerminal, ProgressPrinter) { + term := &ui.MockTerminal{} printer := NewTextProgress(term, 3) return term, printer } @@ -28,31 +18,31 @@ func createTextProgress() (*mockTerm, ProgressPrinter) { func TestPrintUpdate(t *testing.T) { term, printer := createTextProgress() printer.Update(State{3, 11, 0, 29, 47, 0}, 5*time.Second) - test.Equals(t, []string{"[0:05] 61.70% 3 files/dirs 29 B, total 11 files/dirs 47 B"}, term.output) + test.Equals(t, []string{"[0:05] 61.70% 3 files/dirs 29 B, total 11 files/dirs 47 B"}, term.Output) } func TestPrintUpdateWithSkipped(t *testing.T) { term, printer := createTextProgress() printer.Update(State{3, 11, 2, 29, 47, 59}, 5*time.Second) - test.Equals(t, []string{"[0:05] 61.70% 3 files/dirs 29 B, total 11 files/dirs 47 B, skipped 2 files/dirs 59 B"}, term.output) + test.Equals(t, []string{"[0:05] 61.70% 3 files/dirs 29 B, total 11 files/dirs 47 B, skipped 2 files/dirs 59 B"}, term.Output) } func TestPrintSummaryOnSuccess(t *testing.T) { term, printer := createTextProgress() printer.Finish(State{11, 11, 0, 47, 47, 0}, 5*time.Second) - test.Equals(t, []string{"Summary: Restored 11 files/dirs (47 B) in 0:05"}, term.output) + test.Equals(t, []string{"Summary: Restored 11 files/dirs (47 B) in 0:05"}, term.Output) } func TestPrintSummaryOnErrors(t *testing.T) { term, printer := createTextProgress() printer.Finish(State{3, 11, 0, 29, 47, 0}, 5*time.Second) - test.Equals(t, []string{"Summary: Restored 3 / 11 files/dirs (29 B / 47 B) in 0:05"}, term.output) + test.Equals(t, []string{"Summary: Restored 3 / 11 files/dirs (29 B / 47 B) in 0:05"}, term.Output) } func TestPrintSummaryOnSuccessWithSkipped(t *testing.T) { term, printer := createTextProgress() printer.Finish(State{11, 11, 2, 47, 47, 59}, 5*time.Second) - test.Equals(t, []string{"Summary: Restored 11 files/dirs (47 B) in 0:05, skipped 2 files/dirs 59 B"}, term.output) + test.Equals(t, []string{"Summary: Restored 11 files/dirs (47 B) in 0:05, skipped 2 files/dirs 59 B"}, term.Output) } func TestPrintCompleteItem(t *testing.T) { @@ -70,6 +60,12 @@ func TestPrintCompleteItem(t *testing.T) { } { term, printer := createTextProgress() printer.CompleteItem(data.action, "test", data.size) - test.Equals(t, []string{data.expected}, term.output) + test.Equals(t, []string{data.expected}, term.Output) } } + +func TestError(t *testing.T) { + term, printer := createTextProgress() + test.Equals(t, printer.Error("/path", errors.New("error \"message\"")), nil) + test.Equals(t, []string{"ignoring error for /path: error \"message\"\n"}, term.Errors) +} diff --git a/internal/ui/terminal.go b/internal/ui/terminal.go new file mode 100644 index 000000000..2d9418a61 --- /dev/null +++ b/internal/ui/terminal.go @@ -0,0 +1,10 @@ +package ui + +// Terminal is used to write messages and display status lines which can be +// updated. See termstatus.Terminal for a concrete implementation. +type Terminal interface { + Print(line string) + Error(line string) + SetStatus(lines []string) + CanUpdateStatus() bool +} From 88f59fc2d6d40b71e74d8566117d496a9f949203 Mon Sep 17 00:00:00 2001 From: Michael Terry Date: Sat, 3 Aug 2024 15:29:10 -0400 Subject: [PATCH 4/4] json: switch backup and restore errors from string to struct types This keeps backwards compatibility with the previous empty structs. And maybe we'd want to put other fields into the inner struct later, rather than the outer message. --- doc/075_scripting.rst | 15 ++++++++++++++- internal/ui/backup/json.go | 16 ++++++++++------ internal/ui/backup/json_test.go | 4 ++-- internal/ui/restore/json.go | 14 +++++++++----- internal/ui/restore/json_test.go | 2 +- 5 files changed, 36 insertions(+), 15 deletions(-) diff --git a/doc/075_scripting.rst b/doc/075_scripting.rst index 87ae4fcf4..fa7fa1b6e 100644 --- a/doc/075_scripting.rst +++ b/doc/075_scripting.rst @@ -139,7 +139,7 @@ Error +----------------------+-------------------------------------------+ | ``message_type`` | Always "error" | +----------------------+-------------------------------------------+ -| ``error`` | Error message | +| ``error.message`` | Error message | +----------------------+-------------------------------------------+ | ``during`` | What restic was trying to do | +----------------------+-------------------------------------------+ @@ -539,6 +539,19 @@ Status |``bytes_skipped`` | Total size of skipped files | +----------------------+------------------------------------------------------------+ +Error +^^^^^ + ++----------------------+-------------------------------------------+ +| ``message_type`` | Always "error" | ++----------------------+-------------------------------------------+ +| ``error.message`` | Error message | ++----------------------+-------------------------------------------+ +| ``during`` | Always "restore" | ++----------------------+-------------------------------------------+ +| ``item`` | Usually, the path of the problematic file | ++----------------------+-------------------------------------------+ + Verbose Status ^^^^^^^^^^^^^^ diff --git a/internal/ui/backup/json.go b/internal/ui/backup/json.go index bb6685136..f4a76afd7 100644 --- a/internal/ui/backup/json.go +++ b/internal/ui/backup/json.go @@ -67,7 +67,7 @@ func (b *JSONProgress) Update(total, processed Counter, errors uint, currentFile func (b *JSONProgress) ScannerError(item string, err error) error { b.error(errorUpdate{ MessageType: "error", - Error: err.Error(), + Error: errorObject{err.Error()}, During: "scan", Item: item, }) @@ -78,7 +78,7 @@ func (b *JSONProgress) ScannerError(item string, err error) error { func (b *JSONProgress) Error(item string, err error) error { b.error(errorUpdate{ MessageType: "error", - Error: err.Error(), + Error: errorObject{err.Error()}, During: "archival", Item: item, }) @@ -205,11 +205,15 @@ type statusUpdate struct { CurrentFiles []string `json:"current_files,omitempty"` } +type errorObject struct { + Message string `json:"message"` +} + type errorUpdate struct { - MessageType string `json:"message_type"` // "error" - Error string `json:"error"` - During string `json:"during"` - Item string `json:"item"` + MessageType string `json:"message_type"` // "error" + Error errorObject `json:"error"` + During string `json:"during"` + Item string `json:"item"` } type verboseUpdate struct { diff --git a/internal/ui/backup/json_test.go b/internal/ui/backup/json_test.go index 4846279b3..b4872efd5 100644 --- a/internal/ui/backup/json_test.go +++ b/internal/ui/backup/json_test.go @@ -17,11 +17,11 @@ func createJSONProgress() (*ui.MockTerminal, ProgressPrinter) { func TestJSONError(t *testing.T) { term, printer := createJSONProgress() test.Equals(t, printer.Error("/path", errors.New("error \"message\"")), nil) - test.Equals(t, []string{"{\"message_type\":\"error\",\"error\":\"error \\\"message\\\"\",\"during\":\"archival\",\"item\":\"/path\"}\n"}, term.Errors) + test.Equals(t, []string{"{\"message_type\":\"error\",\"error\":{\"message\":\"error \\\"message\\\"\"},\"during\":\"archival\",\"item\":\"/path\"}\n"}, term.Errors) } func TestJSONScannerError(t *testing.T) { term, printer := createJSONProgress() test.Equals(t, printer.ScannerError("/path", errors.New("error \"message\"")), nil) - test.Equals(t, []string{"{\"message_type\":\"error\",\"error\":\"error \\\"message\\\"\",\"during\":\"scan\",\"item\":\"/path\"}\n"}, term.Errors) + test.Equals(t, []string{"{\"message_type\":\"error\",\"error\":{\"message\":\"error \\\"message\\\"\"},\"during\":\"scan\",\"item\":\"/path\"}\n"}, term.Errors) } diff --git a/internal/ui/restore/json.go b/internal/ui/restore/json.go index 4135dd667..72cc38a6e 100644 --- a/internal/ui/restore/json.go +++ b/internal/ui/restore/json.go @@ -48,7 +48,7 @@ func (t *jsonPrinter) Update(p State, duration time.Duration) { func (t *jsonPrinter) Error(item string, err error) error { t.error(errorUpdate{ MessageType: "error", - Error: err.Error(), + Error: errorObject{err.Error()}, During: "restore", Item: item, }) @@ -113,11 +113,15 @@ type statusUpdate struct { BytesSkipped uint64 `json:"bytes_skipped,omitempty"` } +type errorObject struct { + Message string `json:"message"` +} + type errorUpdate struct { - MessageType string `json:"message_type"` // "error" - Error string `json:"error"` - During string `json:"during"` - Item string `json:"item"` + MessageType string `json:"message_type"` // "error" + Error errorObject `json:"error"` + During string `json:"during"` + Item string `json:"item"` } type verboseUpdate struct { diff --git a/internal/ui/restore/json_test.go b/internal/ui/restore/json_test.go index 1e0f80a4f..917a48070 100644 --- a/internal/ui/restore/json_test.go +++ b/internal/ui/restore/json_test.go @@ -66,5 +66,5 @@ func TestJSONPrintCompleteItem(t *testing.T) { func TestJSONError(t *testing.T) { term, printer := createJSONProgress() test.Equals(t, printer.Error("/path", errors.New("error \"message\"")), nil) - test.Equals(t, []string{"{\"message_type\":\"error\",\"error\":\"error \\\"message\\\"\",\"during\":\"restore\",\"item\":\"/path\"}\n"}, term.Errors) + test.Equals(t, []string{"{\"message_type\":\"error\",\"error\":{\"message\":\"error \\\"message\\\"\"},\"during\":\"restore\",\"item\":\"/path\"}\n"}, term.Errors) }