diff --git a/pkg/cli/cli-impl.go b/pkg/cli/cli-impl.go index d83fc77..0eab43b 100644 --- a/pkg/cli/cli-impl.go +++ b/pkg/cli/cli-impl.go @@ -12,6 +12,7 @@ import ( "github.com/sdsc-ordes/quitsh/pkg/component/stage" "github.com/sdsc-ordes/quitsh/pkg/config" "github.com/sdsc-ordes/quitsh/pkg/errors" + errorsfilter "github.com/sdsc-ordes/quitsh/pkg/errors/filter" "github.com/sdsc-ordes/quitsh/pkg/exec/git" fs "github.com/sdsc-ordes/quitsh/pkg/filesystem" "github.com/sdsc-ordes/quitsh/pkg/log" @@ -79,11 +80,7 @@ func (c *cliApp) Run() error { e := c.rootCmd.Execute() if e != nil { - if c.rootArgs.ErrorSummary { - log.ErrorE(e, "Errors occurred.") - } else { - log.Error("Errors occurred, see above.") - } + log.ErrorE(errorsfilter.FilterAlreadyReported(e), "Errors occurred.") } return e diff --git a/pkg/cli/cmd/root/root.go b/pkg/cli/cmd/root/root.go index fa91966..34b69c5 100644 --- a/pkg/cli/cmd/root/root.go +++ b/pkg/cli/cmd/root/root.go @@ -65,9 +65,6 @@ type ( // Enable running targets in parallel. Parallel bool `yaml:"parallel"` - - // Error summary of all errors at the end. - ErrorSummary bool `yaml:"errorSummary"` } Settings struct { diff --git a/pkg/common/stack/stack.go b/pkg/common/stack/stack.go index 5524999..9d37803 100644 --- a/pkg/common/stack/stack.go +++ b/pkg/common/stack/stack.go @@ -25,26 +25,36 @@ func NewStackWithCap[T any](capacity int) Stack[T] { // Pop pops the top element on the stack. // The stack size needs to be greater > 0. func (s *Stack[T]) Pop() T { - debug.Assert(s.Len() != 0, "the stack size is not > 0") - - res := s.stack[len(s.stack)-1] + res := s.Top() s.stack = s.stack[:len(s.stack)-1] return res } -// PopFront pops the bottom level on the stack. -// This method is useful to do Breath-First-Traversal. -// instead of Depth-First-Traversal when using `Pop`. -func (s *Stack[T]) PopFront() T { +// Returns the top element on the stack. +func (s *Stack[T]) Top() T { debug.Assert(s.Len() != 0, "the stack size is not > 0") - res := s.stack[0] + return s.stack[len(s.stack)-1] +} + +// PopBottom pops the bottom level on the stack. +// This method is useful to do Breadth-First Traversal +// instead of Depth-First Traversal when using `Pop`. +func (s *Stack[T]) PopBottom() T { + res := s.Bottom() s.stack = s.stack[1:] return res } +// Returns the bottom element. +func (s *Stack[T]) Bottom() T { + debug.Assert(s.Len() != 0, "the stack size is not > 0") + + return s.stack[0] +} + // Visit travers the stack from top to bottom and applies a function. // If the visitor returns `false` the iteration is aborted. func (s *Stack[T]) Visit(visitor func(int, T) bool) { diff --git a/pkg/common/stack/stack_test.go b/pkg/common/stack/stack_test.go index b7cf4eb..1df4d61 100644 --- a/pkg/common/stack/stack_test.go +++ b/pkg/common/stack/stack_test.go @@ -43,19 +43,19 @@ func TestStackFrontPop(t *testing.T) { stack.Push("b") assert.Equal(t, 2, stack.Len()) - assert.Equal(t, "a", stack.PopFront()) + assert.Equal(t, "a", stack.PopBottom()) assert.Equal(t, 1, stack.Len()) stack.Push("c") assert.Equal(t, 2, stack.Len()) - assert.Equal(t, "b", stack.PopFront()) + assert.Equal(t, "b", stack.PopBottom()) assert.Equal(t, 1, stack.Len()) assert.Equal(t, "c", stack.Pop()) assert.Equal(t, 0, stack.Len()) - assert.Panics(t, func() { _ = stack.PopFront() }) + assert.Panics(t, func() { _ = stack.PopBottom() }) } func TestStackTraverse(t *testing.T) { diff --git a/pkg/dag/execution-order.go b/pkg/dag/execution-order.go index bb1b2cb..60e2de5 100644 --- a/pkg/dag/execution-order.go +++ b/pkg/dag/execution-order.go @@ -377,7 +377,7 @@ func visitNodesBFS( for bfsStack.Len() != 0 { log.Tracef("Current BFS stack:\n%v", formatStack(&bfsStack, true)) - n := bfsStack.PopFront() + n := bfsStack.PopBottom() if !visit(n) { continue } diff --git a/pkg/dag/run-concurrent.go b/pkg/dag/run-concurrent.go index 14069e5..e6097f7 100644 --- a/pkg/dag/run-concurrent.go +++ b/pkg/dag/run-concurrent.go @@ -16,8 +16,8 @@ import ( const MaxCoroutineConcurrency = 10000 -// ExecuteConcurrent executes the DAG concurrent. -func ExecuteConcurrent( +// executeConcurrent executes the DAG concurrent. +func executeConcurrent( targetNodes TargetNodeMap, runnerFactory factory.IFactory, toolchainDispatcher toolchain.IDispatcher, @@ -98,7 +98,7 @@ func ExecuteConcurrent( for _, n := range targetNodes { summary.AddStatus(n.Execution.Runners...) } - summary.statuses.Log() + summary.Log() return summary.allErrors } diff --git a/pkg/dag/run.go b/pkg/dag/run.go index 61cd15d..7b86ad1 100644 --- a/pkg/dag/run.go +++ b/pkg/dag/run.go @@ -52,14 +52,14 @@ func Execute( opts ...ExecuteOption, ) error { if parallel { - return ExecuteConcurrent( + return executeConcurrent( targets, runnerFactory, dispatcher, config, rootDir, opts...) } else { - return ExecuteNormal( + return executeNormal( prios, runnerFactory, dispatcher, @@ -69,9 +69,9 @@ func Execute( } } -// ExecuteNormal executes the DAG non-concurrent. +// executeNormal executes the DAG non-concurrent. // If no dispatcher is given, the toolchain dispatch is not done. -func ExecuteNormal( +func executeNormal( prios Priorities, runnerFactory factory.IFactory, toolchainDispatcher toolchain.IDispatcher, @@ -224,7 +224,7 @@ func executeRunners( rD.node.PropagateExecStatus() } - summary.statuses.Log() + summary.Log() return summary.allErrors } diff --git a/pkg/dag/status.go b/pkg/dag/status.go index 3919962..7d5a5fd 100644 --- a/pkg/dag/status.go +++ b/pkg/dag/status.go @@ -9,6 +9,7 @@ import ( "github.com/sdsc-ordes/quitsh/pkg/component/step" "github.com/sdsc-ordes/quitsh/pkg/component/target" "github.com/sdsc-ordes/quitsh/pkg/errors" + errorsfilter "github.com/sdsc-ordes/quitsh/pkg/errors/filter" "github.com/sdsc-ordes/quitsh/pkg/log" "github.com/sdsc-ordes/quitsh/pkg/runner" ) @@ -46,8 +47,7 @@ func (s *Summary) AddStatus(r ...*RunnerStatus) { } } -// Log prints the log of the summary. -func (s RunnerStatuses) Log() { +func (s RunnerStatuses) log() { var sb strings.Builder sb.WriteString("Summary:\n") @@ -83,3 +83,13 @@ func (s RunnerStatuses) Log() { log.Info(sb.String()) } + +// Log prints the log of the summary. +func (s *Summary) Log() { + s.statuses.log() + + // Add the sentinel error, for later dropping. + if s.allErrors != nil { + s.allErrors = errorsfilter.WrapAsReported(s.allErrors) + } +} diff --git a/pkg/errors/filter/already-reported.go b/pkg/errors/filter/already-reported.go new file mode 100644 index 0000000..791bb5f --- /dev/null +++ b/pkg/errors/filter/already-reported.go @@ -0,0 +1,94 @@ +package errorsfilter + +import ( + "github.com/hashicorp/go-multierror" + "github.com/sdsc-ordes/quitsh/pkg/common/stack" + "github.com/sdsc-ordes/quitsh/pkg/log" +) + +// alreadyReportedError is an wrapping error which can be dropped by `DropReported`. +type alreadyReportedError struct { + e error +} + +func (e *alreadyReportedError) Error() string { + return e.e.Error() +} + +func (e *alreadyReportedError) Unwrap() error { + return e.e +} + +// WrapAsReported returns a new wrapped error which indicates +// that it is already reported. +// Useful for bigger errors, which you still want to propagate upwards but +// maybe ignore in logging then with [FilterAlreadyReported]. +func WrapAsReported(e error) error { + return &alreadyReportedError{e: e} +} + +// FilterAlreadyReported filters `ErrAlreadyReported` from the chain. +// NOTE: This only filters on [multierror.Error] s since, `Unwrap` functionality does not let +// reconstruct the error. We are using [multierror.Error] extensively anyways. +// +//nolint:gocognit,nestif // TODO: later. +func FilterAlreadyReported(err error) error { + if err == nil { + return nil + } + + type Node struct { + parent *Node + e error + + multi *multierror.Error + new error + visited bool // Denotes if we visited the node on down traversal. + } + + stack := stack.NewStack[*Node]() + root := &Node{e: err} + stack.Push(root) + + for stack.Len() != 0 { + node := stack.Top() + if node.visited { + // Backtracking + // Reconstruct errors. + ignore := false + if _, ok := node.e.(*alreadyReportedError); ok { //nolint:errorlint // This is ok. + ignore = true + } + + if !ignore { + // Create our new error from any children we have. + if node.multi != nil { + node.new = node.multi + } else { + node.new = node.e + } + + // Add ourself to parent. + if node.parent != nil { + node.parent.multi.Errors = append(node.parent.multi.Errors, node.new) + } + } else { + log.Debug("Dropping error 'alreadyReportedErr'.") + } + + stack.Pop() + + continue + } + + node.visited = true + if e, ok := node.e.(*multierror.Error); ok { //nolint:errorlint // This is ok. + node.multi = &multierror.Error{} // init a new multi list error + for _, c := range e.Errors { + stack.Push(&Node{e: c, parent: node}) + } + } + } + + return root.new +} diff --git a/pkg/errors/filter/already-reported_test.go b/pkg/errors/filter/already-reported_test.go new file mode 100644 index 0000000..aa07e14 --- /dev/null +++ b/pkg/errors/filter/already-reported_test.go @@ -0,0 +1,94 @@ +package errorsfilter + +import ( + "errors" + "testing" + + "github.com/hashicorp/go-multierror" + "github.com/stretchr/testify/require" +) + +func TestDropReported(t *testing.T) { + var errA = errors.New("error A") + var errB = errors.New("error B") + var errC = errors.New("error C") + var errD = errors.New("error D") + + tests := []struct { + name string + input error + wantNil bool + wantErrs []error // errors expected to survive (checked via errors.Is) + wantDrop []error // errors expected to be dropped + }{ + { + name: "nil input", + input: nil, + wantNil: true, + }, + { + name: "single error passes through", + input: errA, + wantErrs: []error{errA}, + }, + { + name: "multierror without ErrReported passes through", + input: multierror.Append(nil, errA, errB), + wantErrs: []error{errA, errB}, + }, + { + name: "wrapped error is dropped", + input: WrapAsReported(errA), + wantNil: true, + }, + { + name: "flat: two dropped in multierror", + input: multierror.Append(nil, + WrapAsReported(errA), // dropped + errB, + WrapAsReported(multierror.Append(nil, errC)), // dropped + ), + wantErrs: []error{errB}, + wantDrop: []error{errA, errC}, + }, + { + name: "nested: two dropped in multierror", + input: &multierror.Error{ + Errors: []error{ + WrapAsReported(errA), + errB, + &multierror.Error{ + Errors: []error{ + WrapAsReported(errC), + errD, + }, + }, + }, + }, + wantErrs: []error{errB, errD}, + wantDrop: []error{errA, errC}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := FilterAlreadyReported(tt.input) + + if tt.wantNil { + require.NoError(t, result) + + return + } + + require.Error(t, result) + + for _, want := range tt.wantErrs { + require.ErrorIs(t, result, want) + } + + for _, dropped := range tt.wantDrop { + require.NotErrorIs(t, result, dropped) + } + }) + } +}