Skip to content

Commit

Permalink
fix: improve output detail collection for command failures
Browse files Browse the repository at this point in the history
  • Loading branch information
garethgeorge committed Dec 9, 2023
1 parent b9bcc7e commit c492f9b
Show file tree
Hide file tree
Showing 11 changed files with 162 additions and 102 deletions.
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ require (
require (
github.com/daaku/go.zipexe v1.0.2 // indirect
github.com/golang/protobuf v1.5.3 // indirect
github.com/google/renameio v1.0.1 // indirect
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/mattn/go-isatty v0.0.20 // indirect
go.uber.org/multierr v1.11.0 // indirect
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiu
github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/renameio v1.0.1 h1:Lh/jXZmvZxb0BBeSY5VKEfidcbcbenKjZFzM/q0fSeU=
github.com/google/renameio v1.0.1/go.mod h1:t/HQoYBZSsWSNK35C6CO/TpPLDVWvxOHboWUAweKUpk=
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.1 h1:HcUWd006luQPljE73d5sk+/VgYPGUReEVz2y1/qylwY=
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.1/go.mod h1:w9Y7gY31krpLmrVU5ZPG9H7l9fZuRu5/3R3S3FMtVQ4=
github.com/grpc-ecosystem/grpc-gateway/v2 v2.18.1 h1:6UKoz5ujsI55KNpsJH3UwCq3T8kKbZwNZBNPuTTje8U=
Expand Down
3 changes: 0 additions & 3 deletions internal/orchestrator/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,6 @@ func (r *RepoOrchestrator) Backup(ctx context.Context, plan *v1.Plan, progressCa
}

func (r *RepoOrchestrator) ListSnapshotFiles(ctx context.Context, snapshotId string, path string) ([]*v1.LsEntry, error) {
r.mu.Lock()
defer r.mu.Unlock()

_, entries, err := r.repo.ListDirectory(ctx, snapshotId, path)
if err != nil {
return nil, fmt.Errorf("failed to list snapshot files: %w", err)
Expand Down
29 changes: 18 additions & 11 deletions pkg/restic/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ import (
"os/exec"
)

const outputBufferLimit = 1000

type CmdError struct {
Command string
Err error
Output string
Err error
Output string
}

func (e *CmdError) Error() string {
Expand All @@ -28,18 +30,23 @@ func (e *CmdError) Is(target error) bool {
return ok
}

// NewCmdError creates a new error indicating that running a command failed.
func NewCmdError(cmd *exec.Cmd, output []byte, err error) *CmdError {
// newCmdError creates a new error indicating that running a command failed.
func newCmdError(cmd *exec.Cmd, output string, err error) *CmdError {
cerr := &CmdError{
Command: cmd.String(),
Err: err,
Err: err,
}

if len(output) > 0 {
if len(output) > 1000 {
output = output[:1000]
}
cerr.Output = string(output)
if len(output) >= outputBufferLimit {
cerr.Output = output[:outputBufferLimit] + "\n...[truncated]"
}

return cerr
}
}

func newCmdErrorPreformatted(cmd *exec.Cmd, output string, err error) *CmdError {
return &CmdError{
Command: cmd.String(),
Err: err,
}
}
86 changes: 86 additions & 0 deletions pkg/restic/io.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package restic

import (
"fmt"
"io"
)

// headWriter keeps the first 'limit' bytes in memory.
type headWriter struct {
buf []byte
limit int
}

var _ io.Writer = &headWriter{}

func (w *headWriter) Write(p []byte) (n int, err error) {
if len(w.buf) >= w.limit {
return len(p), nil
}
w.buf = append(w.buf, p...)
if len(w.buf) > w.limit {
w.buf = w.buf[:w.limit]
}
return len(p), nil
}

func (w *headWriter) Bytes() []byte {
return w.buf
}

// tailWriter keeps the last 'limit' bytes in memory.
type tailWriter struct {
buf []byte
limit int
}

var _ io.Writer = &tailWriter{}

func (w *tailWriter) Write(p []byte) (n int, err error) {
w.buf = append(w.buf, p...)
if len(w.buf) > w.limit {
w.buf = w.buf[len(w.buf)-w.limit:]
}
return len(p), nil
}

func (w *tailWriter) Bytes() []byte {
return w.buf
}

type outputCapturer struct {
headWriter
tailWriter
limit int
totalBytes int
}

var _ io.Writer = &outputCapturer{}

func newOutputCapturer(limit int) *outputCapturer {
return &outputCapturer{
headWriter: headWriter{limit: limit},
tailWriter: tailWriter{limit: limit},
limit: limit,
}
}

func (w *outputCapturer) Write(p []byte) (n int, err error) {
w.headWriter.Write(p)
w.tailWriter.Write(p)
w.totalBytes += len(p)
return len(p), nil
}

func (w *outputCapturer) String() string {
head := w.headWriter.Bytes()
tail := w.tailWriter.Bytes()
if w.totalBytes <= w.limit {
return string(head)
}

head = head[:w.limit/2]
tail = tail[len(tail)-w.limit/2:]

return fmt.Sprintf("%s...[%v bytes dropped]...%s", string(head), w.totalBytes-len(head)-len(tail), string(tail))
}
24 changes: 24 additions & 0 deletions pkg/restic/io_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package restic

import "testing"

func TestOutputCapture(t *testing.T) {
c := newOutputCapturer(100)

c.Write([]byte("hello"))

if c.String() != "hello" {
t.Errorf("expected 'hello', got '%s'", c.String())
}
}

func TestOutputCaptureDrops(t *testing.T) {
c := newOutputCapturer(2)

c.Write([]byte("hello"))

want := "h...[3 bytes dropped]...o"
if c.String() != want {
t.Errorf("expected '%s', got '%s'", want, c.String())
}
}
34 changes: 0 additions & 34 deletions pkg/restic/limitwriter.go

This file was deleted.

10 changes: 6 additions & 4 deletions pkg/restic/outputs.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func readBackupProgressEntries(cmd *exec.Cmd, output io.Reader, callback func(ev
bytes = append(bytes, scanner.Bytes()...)
}

return nil, NewCmdError(cmd, bytes, fmt.Errorf("command output was not JSON: %w", err))
return nil, newCmdError(cmd, string(bytes), fmt.Errorf("command output was not JSON: %w", err))
}
if err := event.Validate(); err != nil {
return nil, err
Expand Down Expand Up @@ -254,7 +254,7 @@ func readRestoreProgressEntries(cmd *exec.Cmd, output io.Reader, callback func(e
bytes = append(bytes, scanner.Bytes()...)
}

return nil, NewCmdError(cmd, bytes, fmt.Errorf("command output was not JSON: %w", err))
return nil, newCmdError(cmd, string(bytes), fmt.Errorf("command output was not JSON: %w", err))
}
if err := event.Validate(); err != nil {
return nil, err
Expand All @@ -271,10 +271,12 @@ func readRestoreProgressEntries(cmd *exec.Cmd, output io.Reader, callback func(e
for scanner.Scan() {
var event RestoreProgressEntry
if err := json.Unmarshal(scanner.Bytes(), &event); err != nil {
return nil, fmt.Errorf("failed to parse JSON: %w", err)
// skip it. Best effort parsing, restic will return with a non-zero exit code if it fails.
continue
}
if err := event.Validate(); err != nil {
return nil, err
// skip it. Best effort parsing, restic will return with a non-zero exit code if it fails.
continue
}

if callback != nil {
Expand Down
Loading

0 comments on commit c492f9b

Please sign in to comment.