feat: filter reported errors (#70)

- Add `WrapAsReported` / `FilterAlreadyReported` utilities (plus tests) to mark and filter “already reported” errors.
- Wrap `Summary.allErrors` after DAG summary logging, and filter the final CLI error before printing an error summary.
- Rename stack dequeue API (`PopFront` → `PopBottom`) and adjust BFS usage accordingly.

Author: gabyx

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

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 {

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) {

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) {

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
 		}

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
 }

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
 }

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)
+	}
+}

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
+}

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)
+			}
+		})
+	}
+}