Preserve joined rendering error evidence
This commit is contained in:
@@ -1,6 +1,6 @@
|
|||||||
# Invocation reporting audit remediation
|
# Invocation reporting audit remediation
|
||||||
|
|
||||||
Status: remediation in progress; 1 of 7 findings complete.
|
Status: remediation in progress; 2 of 7 findings complete.
|
||||||
|
|
||||||
## Review scope
|
## Review scope
|
||||||
|
|
||||||
@@ -132,7 +132,12 @@ correctly detects missing artifacts or invalid selections.
|
|||||||
|
|
||||||
## 2. Retain independent rendering errors joined with write errors
|
## 2. Retain independent rendering errors joined with write errors
|
||||||
|
|
||||||
Priority: P2. Status: open.
|
Priority: P2. Status: complete.
|
||||||
|
|
||||||
|
- [x] Completed: joined callback errors retain independent rendering evidence,
|
||||||
|
while plain and wrapped sink errors remain write failures. Output and detail
|
||||||
|
regressions pass, including joined destination errors.
|
||||||
|
Validation: `GOTOOLCHAIN=local go test ./...` in the repository root.
|
||||||
|
|
||||||
### Finding and evidence
|
### Finding and evidence
|
||||||
|
|
||||||
|
|||||||
36
reporter.go
36
reporter.go
@@ -3,6 +3,7 @@ package commandkit
|
|||||||
import (
|
import (
|
||||||
"errors"
|
"errors"
|
||||||
"io"
|
"io"
|
||||||
|
"reflect"
|
||||||
"strings"
|
"strings"
|
||||||
"sync"
|
"sync"
|
||||||
|
|
||||||
@@ -475,13 +476,46 @@ func finishRenderLocked(state *reportingState, destination reportingDestination,
|
|||||||
}
|
}
|
||||||
|
|
||||||
func recordRendererErrorLocked(state *reportingState, destination reportingDestination, renderErr, writeErr error) error {
|
func recordRendererErrorLocked(state *reportingState, destination reportingDestination, renderErr, writeErr error) error {
|
||||||
if renderErr == nil || (writeErr != nil && errors.Is(renderErr, writeErr)) {
|
if renderErr == nil || (writeErr != nil && propagatesOnlyWriteError(renderErr, writeErr)) {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
state.recordRenderErrorLocked(destination, renderErr)
|
state.recordRenderErrorLocked(destination, renderErr)
|
||||||
return renderErr
|
return renderErr
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A joined callback error may contain both a writer failure and independent
|
||||||
|
// rendering evidence. Walk each branch once rather than discarding the entire
|
||||||
|
// error merely because errors.Is finds the writer failure somewhere in it.
|
||||||
|
func propagatesOnlyWriteError(err, writeErr error) bool {
|
||||||
|
for err != nil {
|
||||||
|
if reflect.TypeOf(err).Comparable() && err == writeErr {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
switch wrapped := err.(type) {
|
||||||
|
case interface{ Unwrap() []error }:
|
||||||
|
found := false
|
||||||
|
for _, child := range wrapped.Unwrap() {
|
||||||
|
if child != nil {
|
||||||
|
found = true
|
||||||
|
if !propagatesOnlyWriteError(child, writeErr) {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if found {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
case interface{ Unwrap() error }:
|
||||||
|
if child := wrapped.Unwrap(); child != nil {
|
||||||
|
err = child
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return errors.Is(err, writeErr)
|
||||||
|
}
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
func combineOperationErrors(renderErr, writeErr error) error {
|
func combineOperationErrors(renderErr, writeErr error) error {
|
||||||
switch {
|
switch {
|
||||||
case renderErr == nil:
|
case renderErr == nil:
|
||||||
|
|||||||
60
reporter_error_test.go
Normal file
60
reporter_error_test.go
Normal file
@@ -0,0 +1,60 @@
|
|||||||
|
package commandkit
|
||||||
|
|
||||||
|
import (
|
||||||
|
"errors"
|
||||||
|
"fmt"
|
||||||
|
"io"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestReporterPreservesJoinedRendererEvidence(t *testing.T) {
|
||||||
|
for _, kind := range []string{"output", "detail"} {
|
||||||
|
for _, test := range []struct {
|
||||||
|
name string
|
||||||
|
combine func(error, error) error
|
||||||
|
rendering bool
|
||||||
|
}{
|
||||||
|
{"joined", func(render, write error) error { return errors.Join(render, write) }, true},
|
||||||
|
{"wrapped join", func(render, write error) error {
|
||||||
|
return fmt.Errorf("rendering context: %w", errors.Join(write, render))
|
||||||
|
}, true},
|
||||||
|
{"plain write", func(_, write error) error { return write }, false},
|
||||||
|
{"wrapped write", func(_, write error) error { return fmt.Errorf("writing context: %w", write) }, false},
|
||||||
|
{"joined write only", func(_, write error) error { return errors.Join(write, fmt.Errorf("same writer: %w", write)) }, false},
|
||||||
|
} {
|
||||||
|
t.Run(kind+"/"+test.name, func(t *testing.T) {
|
||||||
|
renderErr := errors.New("independent rendering failure")
|
||||||
|
// The destination error can itself be a joined value. Returning that
|
||||||
|
// complete error still propagates a single destination failure.
|
||||||
|
writeErr := errors.Join(errors.New("sink failure"), errors.New("sink detail"))
|
||||||
|
destination := &reporterTestWriter{err: writeErr}
|
||||||
|
reporter := newReporter(Streams{Out: destination, Err: destination})
|
||||||
|
reporter.setVisibility(modeVerbose, false, false)
|
||||||
|
render := func(w io.Writer) error {
|
||||||
|
_, err := io.WriteString(w, "report")
|
||||||
|
return test.combine(renderErr, err)
|
||||||
|
}
|
||||||
|
var err error
|
||||||
|
if kind == "output" {
|
||||||
|
err = reporter.RenderOutput(render)
|
||||||
|
} else {
|
||||||
|
err = reporter.Warn("", render)
|
||||||
|
}
|
||||||
|
if !errors.Is(err, writeErr) || errors.Is(err, renderErr) != test.rendering {
|
||||||
|
t.Fatalf("returned error lost evidence: %v", err)
|
||||||
|
}
|
||||||
|
got := reporter.finalize(Success(), nil)
|
||||||
|
result := got.Presentation.Output
|
||||||
|
if kind == "detail" {
|
||||||
|
result = got.Presentation.Diagnostics
|
||||||
|
}
|
||||||
|
if result.State != DeliveryIncomplete || !errors.Is(result.WriteError, writeErr) || errors.Is(result.RenderError, renderErr) != test.rendering {
|
||||||
|
t.Fatalf("presentation=%#v", result)
|
||||||
|
}
|
||||||
|
if !test.rendering && result.RenderError != nil {
|
||||||
|
t.Fatalf("write propagation became rendering evidence: %v", result.RenderError)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user