From a431f16e6fad74c459f627ef29afc139df0168e4 Mon Sep 17 00:00:00 2001 From: Ophestra Date: Wed, 12 Aug 2026 16:21:47 +0900 Subject: [PATCH] internal/pkg: in-flight error resolution The DCE is a slow and overcomplicated solution to a simple problem. This change replaces the DCE by resolving errors in-flight. Signed-off-by: Ophestra --- cmd/mbf/main.go | 4 ++ internal/pkg/exec_test.go | 7 +-- internal/pkg/pkg.go | 125 ++++++++++---------------------------- internal/pkg/pkg_test.go | 108 ++++++++++++++------------------ 4 files changed, 81 insertions(+), 163 deletions(-) diff --git a/cmd/mbf/main.go b/cmd/mbf/main.go index 2312c818..0024275a 100644 --- a/cmd/mbf/main.go +++ b/cmd/mbf/main.go @@ -1196,6 +1196,10 @@ func main() { if w, ok := err.(interface{ Unwrap() []error }); !ok { log.Fatal(err) } else { + if _, ok = w.(pkg.InputError); ok { + log.Fatal(w) + } + errs := w.Unwrap() for i, e := range errs { if i == len(errs)-1 { diff --git a/internal/pkg/exec_test.go b/internal/pkg/exec_test.go index 28ca2002..222ab790 100644 --- a/internal/pkg/exec_test.go +++ b/internal/pkg/exec_test.go @@ -118,11 +118,8 @@ func TestExec(t *testing.T) { []string{"testtool"}, pkg.MustPath("/proc/nonexistent", false, failingArtifact), - ), nil, nil, pkg.WNew, &pkg.DependencyCureError{ - { - A: failingArtifact, - Err: stub.UniqueError(0xcafe), - }, + ), nil, nil, pkg.WNew, pkg.InputError{ + failingArtifact: stub.UniqueError(0xcafe), }}, {"invalid paths", pkg.NewExec( diff --git a/internal/pkg/pkg.go b/internal/pkg/pkg.go index 8dd6abcc..220f268d 100644 --- a/internal/pkg/pkg.go +++ b/internal/pkg/pkg.go @@ -18,7 +18,6 @@ import ( "maps" "math" "os" - "os/signal" "path/filepath" "runtime" "slices" @@ -654,10 +653,10 @@ type pendingArtifactDep struct { // if curing succeeds. resP *cureRes - // Address of result error slice populated during [Cache.Cure], dereferenced + // Address of result error map populated during [Cache.Cure], dereferenced // after acquiring errsMu if curing fails. No additional action is taken, // [Cache] and its caller are responsible for further error handling. - errs *DependencyCureError + errs InputError // Address of mutex synchronising access to errs. errsMu *sync.Mutex @@ -1721,109 +1720,43 @@ retry: goto retry } -// CureError wraps a non-nil error returned attempting to cure an [Artifact]. -type CureError struct { - A Artifact - Err error -} +// An InputError describes inputs of a [FloodArtifact] which had failed to cure. +type InputError map[Artifact]error -// Unwrap returns the underlying error. -func (e *CureError) Unwrap() error { return e.Err } +// Error returns a user-facing, deterministic text representation of e. +func (e InputError) Error() string { + ir := NewIR() -// Error returns the error message from the underlying Err. -func (e *CureError) Error() string { return e.Err.Error() } - -// A DependencyCureError wraps errors returned while curing dependencies. -type DependencyCureError []*CureError - -// unwrapM recursively expands underlying errors into a caller-supplied map. -func (e *DependencyCureError) unwrapM( - ctx context.Context, - ir *IRCache, - me map[unique.Handle[ID]]*CureError, -) { - for _, err := range *e { - if ctx.Err() != nil { - break - } - - id := ir.Ident(err.A) - if _, ok := me[id]; ok { - continue - } - if _e, ok := err.Err.(*DependencyCureError); ok { - _e.unwrapM(ctx, ir, me) - continue - } - me[id] = err + type input struct { + a Artifact + id unique.Handle[ID] } -} - -// unwrap recursively expands and deduplicates underlying errors. -func (e *DependencyCureError) unwrap( - ctx context.Context, - ir *IRCache, -) DependencyCureError { - me := make(map[unique.Handle[ID]]*CureError) - e.unwrapM(ctx, ir, me) - type ent struct { - id unique.Handle[ID] - err *CureError - } - errs := make([]*ent, 0, len(me)) - for id, err := range me { - errs = append(errs, &ent{id, err}) + p := make([]input, 0, len(e)) + for a := range e { + p = append(p, input{a, ir.Ident(a)}) } var identBuf [2]ID - slices.SortFunc(errs, func(a, b *ent) int { + slices.SortFunc(p, func(a, b input) int { identBuf[0], identBuf[1] = a.id.Value(), b.id.Value() return slices.Compare(identBuf[0][:], identBuf[1][:]) }) - _errs := make(DependencyCureError, len(errs)) - for i, v := range errs { - _errs[i] = v.err - } - return _errs -} - -// Unwrap returns a deduplicated slice of underlying errors. -func (e *DependencyCureError) Unwrap() []error { - ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt) - defer cancel() - - errs := e.unwrap(ctx, NewIR()) - _errs := make([]error, len(errs)) - for i, err := range errs { - _errs[i] = err - } - return _errs -} - -// Error returns a user-facing multiline error message. -func (e *DependencyCureError) Error() string { - ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt) - defer cancel() - - ir := NewIR() - errs := e.unwrap(ctx, ir) - if len(errs) == 0 { - return "invalid dependency cure outcome" - } var buf strings.Builder - buf.WriteString("errors curing dependencies:") - for _, err := range errs { + buf.WriteString("errors curing inputs:") + for _, i := range p { buf.WriteString("\n\t" + - reportName(err.A, ir.Ident(err.A)) + ": " + - err.Error()) - } - if ctx.Err() != nil { - buf.WriteString("\nerror resolution cancelled") + reportName(i.a, i.id) + ": " + + e[i.a].Error()) } return buf.String() } +// Unwrap returns a slice of underlying errors in unspecified order. +func (e InputError) Unwrap() []error { + return slices.AppendSeq(make([]error, 0, len(e)), maps.Values(e)) +} + // enterCure must be called before entering an [Artifact] implementation. func (c *Cache) enterCure(a Artifact, curesExempt bool) error { if c.attr.Notify != nil { @@ -2023,7 +1956,7 @@ func (c *Cache) cureMany( wg.Add(len(inputs)) var mask []bool res := make([]cureRes, len(inputs)) - errs := make(DependencyCureError, 0, len(inputs)) + errs := make(InputError) var errsMu sync.Mutex if shallow { mask = make([]bool, len(inputs)) @@ -2042,13 +1975,13 @@ func (c *Cache) cureMany( continue } } - pending := pendingArtifactDep{d, &res[i], &errs, &errsMu, &wg} + pending := pendingArtifactDep{d, &res[i], errs, &errsMu, &wg} go pending.cure(c) } wg.Wait() if len(errs) > 0 { - return mask, &errs + return mask, errs } for i, p := range res { if shallow && mask[i] { @@ -2651,7 +2584,11 @@ func (pending *pendingArtifactDep) cure(c *Cache) { } pending.errsMu.Lock() - *pending.errs = append(*pending.errs, &CureError{pending.a, err}) + if errs, ok := err.(InputError); ok { + maps.Copy(pending.errs, errs) + } else { + pending.errs[pending.a] = err + } pending.errsMu.Unlock() } diff --git a/internal/pkg/pkg_test.go b/internal/pkg/pkg_test.go index 1313d42c..a6964798 100644 --- a/internal/pkg/pkg_test.go +++ b/internal/pkg/pkg_test.go @@ -11,6 +11,7 @@ import ( "io" "io/fs" "log" + "maps" "net/http" "os" "path/filepath" @@ -861,14 +862,11 @@ func TestCache(t *testing.T) { cure: func(f *pkg.FContext) error { panic("attempting to cure impossible artifact") }, - }, nil, nil, pkg.WNew, &pkg.DependencyCureError{ - { - A: failingFile, - Err: struct { - _ []byte - stub.UniqueError - }{UniqueError: 0xbad}, - }, + }, nil, nil, pkg.WNew, pkg.InputError{ + failingFile: struct { + _ []byte + stub.UniqueError + }{UniqueError: 0xbad}, }}, }) @@ -1949,7 +1947,7 @@ errors during scrub: } } -func TestDependencyCureError(t *testing.T) { +func TestInputError(t *testing.T) { t.Parallel() makeIdent := func(ident ...byte) pkg.Artifact { @@ -1962,56 +1960,24 @@ func TestDependencyCureError(t *testing.T) { testCases := []struct { name string - err pkg.DependencyCureError + err pkg.InputError want string unwrap []error }{ - {"simple", pkg.DependencyCureError{ - {A: makeIdent(0xff, 9), Err: stub.UniqueError(0xbad09)}, - {A: makeIdent(0xff, 0), Err: stub.UniqueError(0xbad00)}, - {A: makeIdent(0xff, 0xf), Err: stub.UniqueError(0xbad0f)}, - {A: makeIdent(0xff, 1), Err: stub.UniqueError(0xbad01)}, - }, `errors curing dependencies: + {"simple", pkg.InputError{ + makeIdent(0xff, 9): stub.UniqueError(0xbad09), + makeIdent(0xff, 0): stub.UniqueError(0xbad00), + makeIdent(0xff, 0xf): stub.UniqueError(0xbad0f), + makeIdent(0xff, 1): stub.UniqueError(0xbad01), + }, `errors curing inputs: _wAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA: unique error 765184 injected by the test suite _wEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA: unique error 765185 injected by the test suite _wkAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA: unique error 765193 injected by the test suite _w8AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA: unique error 765199 injected by the test suite`, []error{ - &pkg.CureError{A: makeIdent(0xff, 0), Err: stub.UniqueError(0xbad00)}, - &pkg.CureError{A: makeIdent(0xff, 1), Err: stub.UniqueError(0xbad01)}, - &pkg.CureError{A: makeIdent(0xff, 9), Err: stub.UniqueError(0xbad09)}, - &pkg.CureError{A: makeIdent(0xff, 0xf), Err: stub.UniqueError(0xbad0f)}, - }}, - - {"dedup", pkg.DependencyCureError{ - {A: makeIdent(0xff, 9), Err: stub.UniqueError(0xbad09)}, - {A: makeIdent(0xff, 0), Err: stub.UniqueError(0xbad00)}, - {A: makeIdent(0xff, 0xfd), Err: &pkg.DependencyCureError{ - {A: makeIdent(0xff, 9), Err: stub.UniqueError(0xbad09)}, - {A: makeIdent(0xff, 0xc), Err: &pkg.DependencyCureError{ - {A: makeIdent(0xff, 0xf), Err: stub.UniqueError(0xbad0f)}, - {A: makeIdent(0xff, 0), Err: stub.UniqueError(0xbad00)}, - }}, - {A: makeIdent(0xff, 0), Err: stub.UniqueError(0xbad00)}, - {A: makeIdent(0xff, 0), Err: stub.UniqueError(0xbad00)}, - }}, - {A: makeIdent(0xff, 0xff), Err: &pkg.DependencyCureError{ - {A: makeIdent(0xff, 9), Err: stub.UniqueError(0xbad09)}, - {A: makeIdent(0xff, 0xc), Err: &pkg.DependencyCureError{ - {A: makeIdent(0xff, 0), Err: stub.UniqueError(0xbad00)}, - }}, - {A: makeIdent(0xff, 0), Err: stub.UniqueError(0xbad00)}, - }}, - {A: makeIdent(0xff, 0xf), Err: stub.UniqueError(0xbad0f)}, - {A: makeIdent(0xff, 1), Err: stub.UniqueError(0xbad01)}, - }, `errors curing dependencies: - _wAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA: unique error 765184 injected by the test suite - _wEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA: unique error 765185 injected by the test suite - _wkAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA: unique error 765193 injected by the test suite - _w8AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA: unique error 765199 injected by the test suite`, []error{ - &pkg.CureError{A: makeIdent(0xff, 0), Err: stub.UniqueError(0xbad00)}, - &pkg.CureError{A: makeIdent(0xff, 1), Err: stub.UniqueError(0xbad01)}, - &pkg.CureError{A: makeIdent(0xff, 9), Err: stub.UniqueError(0xbad09)}, - &pkg.CureError{A: makeIdent(0xff, 0xf), Err: stub.UniqueError(0xbad0f)}, + stub.UniqueError(0xbad00), + stub.UniqueError(0xbad01), + stub.UniqueError(0xbad09), + stub.UniqueError(0xbad0f), }}, } for _, tc := range testCases { @@ -2022,7 +1988,19 @@ func TestDependencyCureError(t *testing.T) { t.Errorf("Error:\n%s\nwant\n%s", got, tc.want) } - if unwrap := tc.err.Unwrap(); !reflect.DeepEqual(unwrap, tc.unwrap) { + unwrap, unwrapM := tc.err.Unwrap(), make(map[error]struct{}) + for _, a := range unwrap { + unwrapM[a] = struct{}{} + } + + wantUnwrapM := make(map[error]struct{}) + for _, a := range tc.unwrap { + wantUnwrapM[a] = struct{}{} + } + + if len(unwrap) != len(unwrapM) || + len(tc.unwrap) != len(wantUnwrapM) || + !maps.Equal(unwrapM, wantUnwrapM) { t.Errorf("Unwrap: %#v, want %#v", unwrap, tc.unwrap) } }) @@ -2055,19 +2033,21 @@ func (a earlyFailureF) Cure(*pkg.FContext) error { func BenchmarkEarlyDCE(b *testing.B) { msg := message.New(log.New(os.Stderr, "dce: ", 0)) msg.SwapVerbose(testing.Verbose()) - c, err := pkg.Open(b.Context(), msg, check.MustAbs(b.TempDir()), nil) - if err != nil { - b.Fatal(err) - } - _, _, err = c.Cure(earlyFailureF(8)) - if !errors.Is(err, stub.UniqueError(0xcafe)) { - b.Fatalf("Cure: error = %v", err) - } - c.Close() - dce := err.(*pkg.DependencyCureError) for b.Loop() { - dce.Unwrap() + b.StopTimer() + c, err := pkg.Open(b.Context(), msg, check.MustAbs(b.TempDir()), nil) + if err != nil { + b.Fatal(err) + } + b.StartTimer() + _, _, err = c.Cure(earlyFailureF(8)) + b.StopTimer() + if !errors.Is(err, stub.UniqueError(0xcafe)) { + b.Fatalf("Cure: error = %v", err) + } + c.Close() + b.StartTimer() } }