diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go index 50ca1a313..5685fff5a 100644 --- a/internals/overlord/overlord.go +++ b/internals/overlord/overlord.go @@ -16,6 +16,7 @@ package overlord import ( + "errors" "fmt" "io" "os" @@ -318,11 +319,10 @@ func (o *Overlord) settle(timeout time.Duration, beforeCleanups func()) error { var errs []error for !done { if timeout > 0 && time.Since(t0) > timeout { - err := fmt.Errorf("Settle is not converging") if len(errs) != 0 { - return &multiError{append(errs, err)} + return newMultiError("settle is not converging", errs) } - return err + return errors.New("settle is not converging") } next := o.ensureTimerReset() err := o.stateEng.Ensure() @@ -355,7 +355,7 @@ func (o *Overlord) settle(timeout time.Duration, beforeCleanups func()) error { } } if len(errs) != 0 { - return &multiError{errs} + return newMultiError("ensure error", errs) } return nil } diff --git a/internals/overlord/stateengine.go b/internals/overlord/stateengine.go index 5575357c8..e4b75d83f 100644 --- a/internals/overlord/stateengine.go +++ b/internals/overlord/stateengine.go @@ -15,7 +15,9 @@ package overlord import ( + "bytes" "errors" + "fmt" "strings" "sync" @@ -23,8 +25,6 @@ import ( "github.com/canonical/pebble/internals/overlord/state" ) -var errStateEngineStopped = errors.New("state engine already stopped") - // StateManager is implemented by types responsible for observing // the system and manipulating it to reflect the desired state. type StateManager interface { @@ -80,22 +80,48 @@ func (se *StateEngine) State() *state.State { return se.state } +// multiError collects multiple errors that affected an operation. type multiError struct { - errs []error + header string + errs []error } -func (e *multiError) Error() string { - if len(e.errs) == 1 { - return e.errs[0].Error() - } +// newMultiError returns a new multiError struct initialized with +// the given format string that explains what operation potentially +// went wrong. multiError can be nested and will render correctly +// in these cases. +func newMultiError(header string, errs []error) error { + return &multiError{header: header, errs: errs} +} + +// Error formats the error string. +func (me *multiError) Error() string { + return me.nestedError(0) +} - errStrings := make([]string, len(e.errs)) - for i, err := range e.errs { - errStrings[i] = err.Error() +// helper to ensure formating of nested multiErrors works. +func (me *multiError) nestedError(level int) string { + indent := strings.Repeat(" ", level) + buf := bytes.NewBufferString(fmt.Sprintf("%s:\n", me.header)) + if level > 8 { + return "circular or too deep error nesting (max 8)?!" } - return "multiple errors: " + strings.Join(errStrings, "; ") + for i, err := range me.errs { + switch v := err.(type) { + case *multiError: + fmt.Fprintf(buf, "%s- %v", indent, v.nestedError(level+1)) + default: + fmt.Fprintf(buf, "%s- %v", indent, err) + } + if i < len(me.errs)-1 { + fmt.Fprintf(buf, "\n") + } + } + return buf.String() } +var errStateEngineStopped = errors.New("state engine already stopped") + // DryStart ensures all managers are ready to run their activities // before the first call to Ensure() is made. func (se *StateEngine) DryStart() error { @@ -112,7 +138,7 @@ func (se *StateEngine) DryStart() error { } } if len(errs) != 0 { - return &multiError{errs} + return newMultiError("dry-start failed", errs) } se.dryStarted = true return nil @@ -144,7 +170,7 @@ func (se *StateEngine) Ensure() error { } } if len(errs) != 0 { - return &multiError{errs} + return newMultiError("state ensure errors", errs) } return nil }