From 852bdfcbd3f0acc074a8665e861a440d70c9a850 Mon Sep 17 00:00:00 2001 From: Uwe Krueger Date: Sun, 30 Oct 2022 17:11:48 +0100 Subject: [PATCH] support for deferred error propagation (#167) A typical problem is to handle errors provided by deferred functions, for example Close(). Another scenario is the need to wrap all errors provided in a function with a common context description. The errors package now provides a function PropagateError, which can be used to solve both problems. If takes an optional error context, which will be used to wrap a potential error returned by the calling function. Additionally it is possible to add a function providing an error. This error, if present is composed with an error provided by the function return when called as deferred function. Example: func() (efferr error) { ... defer errors.PropagateErrorf(&efferr, stream.Close, "error context") ... return err } This error providing function may be the Finalize() method of a utils.Finalizer. This is now directly supported by the Finalizer. Example: func() (efferr error) { var finalize utils.Finalizer ... defer finalize.FinalizeWithErrorPropagationf(&efferr, "error context") ... finalize.Close(stream) ... return err } --- .../ocmcmds/common/cmds/signing/cmd.go | 2 +- .../ocmcmds/components/sign/cmd_test.go | 8 +- pkg/common/history.go | 4 + pkg/contexts/config/config/context_test.go | 2 +- pkg/contexts/ocm/signing/handle.go | 67 +++++---- pkg/contexts/ocm/signing/signing_test.go | 4 +- pkg/errors/alreadyexists.go | 10 +- pkg/errors/closed.go | 8 +- pkg/errors/error.go | 53 +++++++- pkg/errors/errprop.go | 24 ++++ pkg/errors/errprop_test.go | 76 +++++++++++ pkg/errors/invalid.go | 10 +- pkg/errors/list.go | 21 ++- pkg/errors/list_test.go | 73 ++++++++++ pkg/errors/notfound.go | 10 +- pkg/errors/notimpl.go | 8 +- pkg/errors/notsupported.go | 8 +- pkg/errors/readonly.go | 8 +- pkg/errors/recursion.go | 18 +-- pkg/errors/unknown.go | 8 +- pkg/utils/finalizer.go | 66 ++++++++- pkg/utils/finalizer_test.go | 127 ++++++++++++++++++ .../{utils_suite_test.go => suite_test.go} | 0 23 files changed, 522 insertions(+), 93 deletions(-) create mode 100644 pkg/errors/errprop.go create mode 100644 pkg/errors/errprop_test.go create mode 100644 pkg/errors/list_test.go rename pkg/utils/{utils_suite_test.go => suite_test.go} (100%) diff --git a/cmds/ocm/commands/ocmcmds/common/cmds/signing/cmd.go b/cmds/ocm/commands/ocmcmds/common/cmds/signing/cmd.go index afbf81225..c20308298 100644 --- a/cmds/ocm/commands/ocmcmds/common/cmds/signing/cmd.go +++ b/cmds/ocm/commands/ocmcmds/common/cmds/signing/cmd.go @@ -122,7 +122,7 @@ func (a *action) Add(e interface{}) error { cv := o.ComponentVersion sopts := *a.sopts sopts.Resolver = ocm.NewCompoundResolver(o.Repository, a.sopts.Resolver) - d, err := signing.Apply(a.printer, &a.state, cv, &sopts) + d, err := signing.Apply(a.printer, &a.state, cv, &sopts, true) a.errlist.Add(err) if err == nil { a.printer.Printf("successfully %s %s:%s (digest %s:%s)\n", a.desc[0], cv.GetName(), cv.GetVersion(), d.HashAlgorithm, d.Value) diff --git a/cmds/ocm/commands/ocmcmds/components/sign/cmd_test.go b/cmds/ocm/commands/ocmcmds/components/sign/cmd_test.go index c30eb260d..3c1d9a029 100644 --- a/cmds/ocm/commands/ocmcmds/components/sign/cmd_test.go +++ b/cmds/ocm/commands/ocmcmds/components/sign/cmd_test.go @@ -187,7 +187,7 @@ successfully signed github.com/mandelsoft/ref:v1 (digest sha256:` + digest + `) buf := bytes.NewBuffer(nil) Expect(env.CatchErrorOutput(buf).Execute("sign", "components", "-s", SIGNATURE, "-K", PRIVKEY, "--repo", ARCH, COMPONENTB+":"+VERSION)).To(HaveOccurred()) Expect(buf.String()).To(StringEqualTrimmedWithContext(` -Error: {signing: failed resolving component reference ref[github.com/mandelsoft/test:v1] in github.com/mandelsoft/ref:v1: ocm reference "github.com/mandelsoft/test:v1" not found} +Error: signing: github.com/mandelsoft/ref:v1: failed resolving component reference ref[github.com/mandelsoft/test:v1]: ocm reference "github.com/mandelsoft/test:v1" not found `)) }) @@ -195,7 +195,7 @@ Error: {signing: failed resolving component reference ref[github.com/mandelsoft/ buf := bytes.NewBuffer(nil) Expect(env.CatchErrorOutput(buf).Execute("sign", "components", "-s", SIGNATURE, "-K", PRIVKEY, ARCH)).To(HaveOccurred()) Expect(buf.String()).To(StringEqualTrimmedWithContext(` -Error: {signing: failed resolving component reference ref[github.com/mandelsoft/test:v1] in github.com/mandelsoft/ref:v1: ocm reference "github.com/mandelsoft/test:v1" not found} +Error: signing: github.com/mandelsoft/ref:v1: failed resolving component reference ref[github.com/mandelsoft/test:v1]: ocm reference "github.com/mandelsoft/test:v1" not found `)) }) }) @@ -215,7 +215,7 @@ Error: {signing: failed resolving component reference ref[github.com/mandelsoft/ buf := bytes.NewBuffer(nil) Expect(env.CatchErrorOutput(buf).Execute("sign", "components", "-s", SIGNATURE, "-K", PRIVKEY, "--repo", ARCH, COMPONENTB+":"+VERSION)).To(HaveOccurred()) Expect(buf.String()).To(StringEqualTrimmedWithContext(` -Error: {signing: failed resolving component reference ref[github.com/mandelsoft/test:v1] in github.com/mandelsoft/ref:v1: ocm reference "github.com/mandelsoft/test:v1" not found} +Error: signing: github.com/mandelsoft/ref:v1: failed resolving component reference ref[github.com/mandelsoft/test:v1]: ocm reference "github.com/mandelsoft/test:v1" not found `)) }) @@ -223,7 +223,7 @@ Error: {signing: failed resolving component reference ref[github.com/mandelsoft/ buf := bytes.NewBuffer(nil) Expect(env.CatchErrorOutput(buf).Execute("sign", "components", "-s", SIGNATURE, "-K", PRIVKEY, ARCH)).To(HaveOccurred()) Expect(buf.String()).To(StringEqualTrimmedWithContext(` -Error: {signing: failed resolving component reference ref[github.com/mandelsoft/test:v1] in github.com/mandelsoft/ref:v1: ocm reference "github.com/mandelsoft/test:v1" not found} +Error: signing: github.com/mandelsoft/ref:v1: failed resolving component reference ref[github.com/mandelsoft/test:v1]: ocm reference "github.com/mandelsoft/test:v1" not found `)) }) }) diff --git a/pkg/common/history.go b/pkg/common/history.go index e7b896b19..323d0283b 100644 --- a/pkg/common/history.go +++ b/pkg/common/history.go @@ -67,6 +67,9 @@ func (h History) Equals(o History) bool { return true } +// Add provided a new extended non-cyclic history. +// If the new entry would lead to a cycle an appropriate +// error is returned. func (h *History) Add(kind string, nv NameVersion) error { if h.Contains(nv) { return errors.ErrRecusion(kind, nv, *h) @@ -75,6 +78,7 @@ func (h *History) Add(kind string, nv NameVersion) error { return nil } +// Append provides a new extended history without cycle check. func (h History) Append(nv ...NameVersion) History { result := make(History, len(h)+len(nv)) copy(result, h) diff --git a/pkg/contexts/config/config/context_test.go b/pkg/contexts/config/config/context_test.go index 08475e639..144819847 100644 --- a/pkg/contexts/config/config/context_test.go +++ b/pkg/contexts/config/config/context_test.go @@ -62,7 +62,7 @@ var _ = Describe("generic config handling", func() { err = cfgctx.ApplyConfig(cfg, "testconfig") Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(Equal("testconfig: {applying generic config list: config entry 0--testconfig: config type \"Dummy\" is unknown, config entry 1--testconfig: config type \"Dummy\" is unknown}")) + Expect(err.Error()).To(Equal("testconfig: applying generic config list: {config entry 0--testconfig: config type \"Dummy\" is unknown, config entry 1--testconfig: config type \"Dummy\" is unknown}")) gen, cfgs := cfgctx.GetConfig(config.AllGenerations, nil) Expect(gen).To(Equal(int64(3))) Expect(len(cfgs)).To(Equal(3)) diff --git a/pkg/contexts/ocm/signing/handle.go b/pkg/contexts/ocm/signing/handle.go index b5b1f7098..fc44cb39b 100644 --- a/pkg/contexts/ocm/signing/handle.go +++ b/pkg/contexts/ocm/signing/handle.go @@ -15,6 +15,7 @@ import ( metav1 "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/meta/v1" "github.com/open-component-model/ocm/pkg/contexts/ocm/cpi" "github.com/open-component-model/ocm/pkg/errors" + "github.com/open-component-model/ocm/pkg/utils" ) func ToDigestSpec(v interface{}) *metav1.DigestSpec { @@ -24,7 +25,7 @@ func ToDigestSpec(v interface{}) *metav1.DigestSpec { return v.(*metav1.DigestSpec) } -func Apply(printer common.Printer, state *common.WalkingState, cv ocm.ComponentVersionAccess, opts *Options) (*metav1.DigestSpec, error) { +func Apply(printer common.Printer, state *common.WalkingState, cv ocm.ComponentVersionAccess, opts *Options, closecv ...bool) (*metav1.DigestSpec, error) { if printer == nil { printer = common.NewPrinter(nil) } @@ -32,15 +33,24 @@ func Apply(printer common.Printer, state *common.WalkingState, cv ocm.ComponentV s := common.NewWalkingState() state = &s } - return apply(printer, *state, cv, opts) + return apply(printer, *state, cv, opts, utils.Optional(closecv...)) } -func apply(printer common.Printer, state common.WalkingState, cv ocm.ComponentVersionAccess, opts *Options) (*metav1.DigestSpec, error) { +func apply(printer common.Printer, state common.WalkingState, cv ocm.ComponentVersionAccess, opts *Options, closecv bool) (d *metav1.DigestSpec, efferr error) { + var closer errors.ErrorFunction + if closecv { + closer = cv.Close + } nv := common.VersionedElementKey(cv) + defer errors.PropagateErrorf(&efferr, closer, "%s", state.History.Append(nv)) + if ok, err := state.Add(ocm.KIND_COMPONENTVERSION, nv); !ok { return ToDigestSpec(state.Closure[nv]), err } + return _apply(printer, state, nv, cv, opts) +} +func _apply(printer common.Printer, state common.WalkingState, nv common.NameVersion, cv ocm.ComponentVersionAccess, opts *Options) (*metav1.DigestSpec, error) { cd := cv.GetDescriptor().Copy() octx := cv.GetContext() printer.Printf("applying to version %q...\n", nv) @@ -51,14 +61,14 @@ func apply(printer common.Printer, state common.WalkingState, cv ocm.ComponentVe signatureNames = append(signatureNames, s.Name) } if len(signatureNames) == 0 && opts.DoVerify() { - return nil, errors.Newf("no signature found in %s", state.History) + return nil, errors.Newf("no signature found") } } if opts.DoVerify() && !opts.DoSign() { for _, n := range signatureNames { f := cd.GetSignatureIndex(n) if f < 0 { - return nil, errors.Newf("signature %q not found in %s", n, state.History) + return nil, errors.Newf("signature %q not found", n) } } } @@ -71,19 +81,18 @@ func apply(printer common.Printer, state common.WalkingState, cv ocm.ComponentVe if reference.Digest == nil || opts.Recursively || opts.Verify { nested, err := opts.Resolver.LookupComponentVersion(reference.GetComponentName(), reference.GetVersion()) if err != nil { - return nil, errors.Wrapf(err, refMsg(reference, state, "failed resolving component reference")) + return nil, errors.Wrapf(err, refMsg(reference, "failed resolving component reference")) } closer := accessio.OnceCloser(nested) defer closer.Close() digestOpts, err := opts.For(reference.Digest) if err != nil { - return nil, errors.Wrapf(err, refMsg(reference, state, "failed resolving hasher for existing digest for component reference")) + return nil, errors.Wrapf(err, refMsg(reference, "failed resolving hasher for existing digest for component reference")) } - calculatedDigest, err = apply(printer.AddGap(" "), state, nested, digestOpts) + calculatedDigest, err = apply(printer.AddGap(" "), state, nested, digestOpts, true) if err != nil { - return nil, errors.Wrapf(err, refMsg(reference, state, "failed applying to component reference")) + return nil, errors.Wrapf(err, refMsg(reference, "failed applying to component reference")) } - closer.Close() } else { printer.Printf(" accepting digest from reference %s", reference) calculatedDigest = reference.Digest @@ -92,7 +101,7 @@ func apply(printer common.Printer, state common.WalkingState, cv ocm.ComponentVe if reference.Digest == nil { cd.References[i].Digest = calculatedDigest } else if calculatedDigest != nil && !reflect.DeepEqual(reference.Digest, calculatedDigest) { - return nil, errors.Newf(refMsg(reference, state, "calculated reference digest (%+v) mismatches existing digest (%+v) for", calculatedDigest, reference.Digest)) + return nil, errors.Newf(refMsg(reference, "calculated reference digest (%+v) mismatches existing digest (%+v) for", calculatedDigest, reference.Digest)) } printer.Printf(" reference %d: %s:%s: digest %s\n", i, reference.ComponentName, reference.Version, calculatedDigest) } @@ -102,7 +111,7 @@ func apply(printer common.Printer, state common.WalkingState, cv ocm.ComponentVe raw := &cd.Resources[i] acc, err := res.Access() if err != nil { - return nil, errors.Wrapf(err, resMsg(raw, "", state, "failed getting access for resource")) + return nil, errors.Wrapf(err, resMsg(raw, "", "failed getting access for resource")) } if _, ok := opts.SkipAccessTypes[acc.GetKind()]; ok { // set the do not sign digest notation on skip-access-type resources @@ -116,7 +125,7 @@ func apply(printer common.Printer, state common.WalkingState, cv ocm.ComponentVe meth, err := acc.AccessMethod(cv) if err != nil { - return nil, errors.Wrapf(err, resMsg(raw, acc.Describe(octx), state, "failed creating access for resource")) + return nil, errors.Wrapf(err, resMsg(raw, acc.Describe(octx), "failed creating access for resource")) } var req []cpi.DigesterType if raw.Digest != nil { @@ -129,20 +138,20 @@ func apply(printer common.Printer, state common.WalkingState, cv ocm.ComponentVe } digest, err := blobdigesters.DetermineDigests(res.Meta().GetType(), opts.Hasher, opts.Registry, meth, req...) if err != nil { - return nil, errors.Wrapf(err, resMsg(raw, acc.Describe(octx), state, "failed determining digest for resource")) + return nil, errors.Wrapf(err, resMsg(raw, acc.Describe(octx), "failed determining digest for resource")) } if len(digest) == 0 { - return nil, errors.Newf(resMsg(raw, acc.Describe(octx), state, "no digester accepts resource")) + return nil, errors.Newf(resMsg(raw, acc.Describe(octx), "no digester accepts resource")) } if raw.Digest != nil && !reflect.DeepEqual(*raw.Digest, digest[0]) { - return nil, errors.Newf(resMsg(raw, acc.Describe(octx), state, "calculated resource digest (%+v) mismatches existing digest (%+v) for", digest, raw.Digest)) + return nil, errors.Newf(resMsg(raw, acc.Describe(octx), "calculated resource digest (%+v) mismatches existing digest (%+v) for", digest, raw.Digest)) } cd.Resources[i].Digest = &digest[0] printer.Printf(" resource %d: %s: digest %s\n", i, res.Meta().GetIdentity(cv.GetDescriptor().Resources), &digest[0]) } digest, err := compdesc.Hash(cd, opts.NormalizationAlgo, opts.Hasher.Create()) if err != nil { - return nil, errors.Wrapf(err, "failed hashing component descriptor %s ", state.History) + return nil, errors.Wrapf(err, "failed hashing component descriptor") } spec := &metav1.DigestSpec{ HashAlgorithm: opts.Hasher.Algorithm(), @@ -160,7 +169,7 @@ func apply(printer common.Printer, state common.WalkingState, cv ocm.ComponentVe pub := opts.PublicKey(n) if pub == nil { if opts.SignatureConfigured(n) { - return nil, errors.ErrNotFound(compdesc.KIND_PUBLIC_KEY, n, state.History.String()) + return nil, errors.ErrNotFound(compdesc.KIND_PUBLIC_KEY, n) } printer.Printf("Warning: no public key for signature %q in %s\n", n, state.History) continue @@ -169,24 +178,24 @@ func apply(printer common.Printer, state common.WalkingState, cv ocm.ComponentVe verifier := opts.Registry.GetVerifier(sig.Signature.Algorithm) if verifier == nil { if opts.SignatureConfigured(n) { - return nil, errors.ErrUnknown(compdesc.KIND_VERIFY_ALGORITHM, n, state.History.String()) + return nil, errors.ErrUnknown(compdesc.KIND_VERIFY_ALGORITHM, n) } printer.Printf("Warning: no verifier (%s) found for signature %q in %s\n", sig.Signature.Algorithm, n, state.History) continue } hasher := opts.Registry.GetHasher(sig.Digest.HashAlgorithm) if hasher == nil { - return nil, errors.ErrUnknown(compdesc.KIND_HASH_ALGORITHM, sig.Digest.HashAlgorithm, state.History.String()) + return nil, errors.ErrUnknown(compdesc.KIND_HASH_ALGORITHM, sig.Digest.HashAlgorithm) } err = verifier.Verify(sig.Digest.Value, hasher.Crypto(), sig.ConvertToSigning(), pub) if err != nil { - return nil, errors.ErrInvalidWrap(err, compdesc.KIND_SIGNATURE, sig.Signature.Algorithm, state.History.String()) + return nil, errors.ErrInvalidWrap(err, compdesc.KIND_SIGNATURE, sig.Signature.Algorithm) } found = append(found, n) } if len(found) == 0 { if !opts.DoSign() { - return nil, errors.Newf("no verifiable signature found in %s", state.History) + return nil, errors.Newf("no verifiable signature found") } } } @@ -195,11 +204,11 @@ func apply(printer common.Printer, state common.WalkingState, cv ocm.ComponentVe if opts.DoSign() && (!opts.DoVerify() || found == -1) { sig, err := opts.Signer.Sign(digest, opts.Hasher.Crypto(), opts.Issuer, opts.PrivateKey()) if err != nil { - return nil, errors.Wrapf(err, "failed signing component descriptor %s ", state.History) + return nil, errors.Wrapf(err, "failed signing component descriptor") } if sig.Issuer != "" { if opts.Issuer != "" && opts.Issuer != sig.Issuer { - return nil, errors.Newf("signature issuer %q does not match intended issuer %q in %s", sig.Issuer, opts.Issuer, state.History) + return nil, errors.Newf("signature issuer %q does not match intended issuer %q", sig.Issuer, opts.Issuer) } } else { sig.Issuer = opts.Issuer @@ -236,13 +245,13 @@ func apply(printer common.Printer, state common.WalkingState, cv ocm.ComponentVe return spec, nil } -func refMsg(ref compdesc.ComponentReference, state common.WalkingState, msg string, args ...interface{}) string { - return fmt.Sprintf("%s %s in %s", fmt.Sprintf(msg, args...), ref, state.History) +func refMsg(ref compdesc.ComponentReference, msg string, args ...interface{}) string { + return fmt.Sprintf("%s %s", fmt.Sprintf(msg, args...), ref) } -func resMsg(ref *compdesc.Resource, acc string, state common.WalkingState, msg string, args ...interface{}) string { +func resMsg(ref *compdesc.Resource, acc string, msg string, args ...interface{}) string { if acc != "" { - return fmt.Sprintf("%s %s:%s (%s) in %s", fmt.Sprintf(msg, args...), ref.Name, ref.Version, acc, state.History) + return fmt.Sprintf("%s %s:%s (%s)", fmt.Sprintf(msg, args...), ref.Name, ref.Version, acc) } - return fmt.Sprintf("%s %s:%s in %s", fmt.Sprintf(msg, args...), ref.Name, ref.Version, state.History) + return fmt.Sprintf("%s %s:%s", fmt.Sprintf(msg, args...), ref.Name, ref.Version) } diff --git a/pkg/contexts/ocm/signing/signing_test.go b/pkg/contexts/ocm/signing/signing_test.go index 9b7d2abed..6ba6391cc 100644 --- a/pkg/contexts/ocm/signing/signing_test.go +++ b/pkg/contexts/ocm/signing/signing_test.go @@ -263,7 +263,7 @@ var _ = Describe("access method", func() { _, err = Apply(nil, nil, cv, opts) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(Equal("no signature found in github.com/mandelsoft/test:v1")) + Expect(err.Error()).To(StringEqualWithContext("github.com/mandelsoft/test:v1: no signature found")) }) }) @@ -303,7 +303,7 @@ var _ = Describe("access method", func() { _, err = Apply(nil, nil, cv, opts) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(Equal("failed resolving component reference ref[github.com/mandelsoft/test:v1] in github.com/mandelsoft/ref:v1: component version \"github.com/mandelsoft/test:v1\" not found: oci artefact \"v1\" not found in component-descriptors/github.com/mandelsoft/test")) + Expect(err.Error()).To(StringEqualWithContext("github.com/mandelsoft/ref:v1: failed resolving component reference ref[github.com/mandelsoft/test:v1]: component version \"github.com/mandelsoft/test:v1\" not found: oci artefact \"v1\" not found in component-descriptors/github.com/mandelsoft/test")) }) }) }) diff --git a/pkg/errors/alreadyexists.go b/pkg/errors/alreadyexists.go index a8bc016cc..655ffa22e 100644 --- a/pkg/errors/alreadyexists.go +++ b/pkg/errors/alreadyexists.go @@ -4,26 +4,26 @@ package errors -type errAlreadyExists struct { +type AlreadyExistsError struct { errinfo } var formatAlreadyExists = NewDefaultFormatter("", "already exists", "in") func ErrAlreadyExists(spec ...string) error { - return &errAlreadyExists{newErrInfo(formatAlreadyExists, spec...)} + return &AlreadyExistsError{newErrInfo(formatAlreadyExists, spec...)} } func ErrAlreadyExistsWrap(err error, spec ...string) error { - return &errAlreadyExists{wrapErrInfo(err, formatAlreadyExists, spec...)} + return &AlreadyExistsError{wrapErrInfo(err, formatAlreadyExists, spec...)} } func IsErrAlreadyExists(err error) bool { - return IsA(err, &errAlreadyExists{}) + return IsA(err, &AlreadyExistsError{}) } func IsErrAlreadyExistsKind(err error, kind string) bool { - var uerr *errNotFound + var uerr *NotFoundError if err == nil || !As(err, &uerr) { return false } diff --git a/pkg/errors/closed.go b/pkg/errors/closed.go index b5013b999..6153d535c 100644 --- a/pkg/errors/closed.go +++ b/pkg/errors/closed.go @@ -4,22 +4,22 @@ package errors -type errClosed struct { +type ClosedError struct { errinfo } var formatClosed = NewDefaultFormatter("is", "closed", "for") func ErrClosed(spec ...string) error { - return &errClosed{newErrInfo(formatClosed, spec...)} + return &ClosedError{newErrInfo(formatClosed, spec...)} } func IsErrClosed(err error) bool { - return IsA(err, &errClosed{}) + return IsA(err, &ClosedError{}) } func IsErrClosedKind(err error, kind string) bool { - var uerr *errClosed + var uerr *ClosedError if err == nil || !As(err, &uerr) { return false } diff --git a/pkg/errors/error.go b/pkg/errors/error.go index 8be53a9cc..304375261 100644 --- a/pkg/errors/error.go +++ b/pkg/errors/error.go @@ -2,6 +2,7 @@ // // SPDX-License-Identifier: Apache-2.0 +// //nolint: errorlint // this is the new As method, also handling error lists package errors import ( @@ -13,15 +14,56 @@ import ( var ( New = errors.New Unwrap = errors.Unwrap - Is = errors.Is - As = errors.As ) func Newf(msg string, args ...interface{}) error { return New(fmt.Sprintf(msg, args...)) } +func As(err error, target any) bool { + if errors.As(err, target) { + return true + } + + for err != nil { + if list, ok := err.(*ErrorList); ok { + for _, n := range list.errors { + if As(n, target) { + return true + } + } + } + err = Unwrap(err) + } + return false +} + +func Is(err error, target error) bool { + if target == nil || err == nil { + return err == target + } + + if errors.Is(err, target) { + return true + } + + for err != nil { + if list, ok := err.(*ErrorList); ok { + for _, n := range list.errors { + if Is(n, target) { + return true + } + } + } + err = Unwrap(err) + } + return false +} + func IsA(err error, target error) bool { + if target == nil { + return err == target + } if err == nil { return false } @@ -31,6 +73,13 @@ func IsA(err error, target error) bool { if reflect.TypeOf(err).AssignableTo(typ) { return true } + if list, ok := err.(*ErrorList); ok { + for _, n := range list.errors { + if IsA(n, target) { + return true + } + } + } err = Unwrap(err) } return false diff --git a/pkg/errors/errprop.go b/pkg/errors/errprop.go new file mode 100644 index 000000000..0518e36b2 --- /dev/null +++ b/pkg/errors/errprop.go @@ -0,0 +1,24 @@ +// SPDX-FileCopyrightText: 2022 SAP SE or an SAP affiliate company and Open Component Model contributors. +// +// SPDX-License-Identifier: Apache-2.0 + +package errors + +type ErrorFunction func() error + +// PropagateError propagates a deferred error to the named return value +// whose address has to be passed as argument. +func PropagateError(errp *error, f ErrorFunction) { + PropagateErrorf(errp, f, "") +} + +// PropagateErrorf propagates an optional deferred error to the named return value +// whose address has to be passed as argument. +// All errors, including the original one, are wrapped by the given context. +func PropagateErrorf(errp *error, f ErrorFunction, msg string, args ...interface{}) { + if f == nil { + *errp = ErrListf(msg, args...).Add(*errp).Result() + } else { + *errp = ErrListf(msg, args...).Add(*errp, f()).Result() + } +} diff --git a/pkg/errors/errprop_test.go b/pkg/errors/errprop_test.go new file mode 100644 index 000000000..0cc1eaa21 --- /dev/null +++ b/pkg/errors/errprop_test.go @@ -0,0 +1,76 @@ +// SPDX-FileCopyrightText: 2022 SAP SE or an SAP affiliate company and Open Component Model contributors. +// +// SPDX-License-Identifier: Apache-2.0 + +package errors_test + +import ( + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/open-component-model/ocm/pkg/errors" +) + +func errfunc(succeed bool) func() error { + if succeed { + return func() error { return nil } + } + return func() error { return fmt.Errorf("error occurred") } +} + +func testFunc(msg string, err error, succeed bool) (efferr error) { + defer errors.PropagateErrorf(&efferr, errfunc(succeed), msg) + return err +} + +var _ = Describe("finalizer", func() { + Context("without context", func() { + It("succeeds", func() { + Expect(testFunc("", nil, true)).To(Succeed()) + }) + + It("fails ", func() { + err := testFunc("", fmt.Errorf("failed"), true) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("failed")) + }) + + It("succeeds with failing finalizer", func() { + err := testFunc("", nil, false) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("error occurred")) + }) + + It("fails with failing finalizer", func() { + err := testFunc("", fmt.Errorf("failed"), false) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("{failed, error occurred}")) + }) + }) + + Context("with context", func() { + It("succeeds", func() { + Expect(testFunc("context", nil, true)).To(Succeed()) + }) + + It("fails ", func() { + err := testFunc("context", fmt.Errorf("failed"), true) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("context: failed")) + }) + + It("succeeds with failing finalizer", func() { + err := testFunc("context", nil, false) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("context: error occurred")) + }) + + It("fails with failing finalizer", func() { + err := testFunc("context", fmt.Errorf("failed"), false) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("context: {failed, error occurred}")) + }) + }) +}) diff --git a/pkg/errors/invalid.go b/pkg/errors/invalid.go index 6abddd2f8..7288c4906 100644 --- a/pkg/errors/invalid.go +++ b/pkg/errors/invalid.go @@ -4,26 +4,26 @@ package errors -type errInvalid struct { +type InvalidError struct { errinfo } var formatInvalid = NewDefaultFormatter("is", "invalid", "for") func ErrInvalid(spec ...string) error { - return &errInvalid{newErrInfo(formatInvalid, spec...)} + return &InvalidError{newErrInfo(formatInvalid, spec...)} } func ErrInvalidWrap(err error, spec ...string) error { - return &errInvalid{wrapErrInfo(err, formatInvalid, spec...)} + return &InvalidError{wrapErrInfo(err, formatInvalid, spec...)} } func IsErrInvalid(err error) bool { - return IsA(err, &errInvalid{}) + return IsA(err, &InvalidError{}) } func IsErrInvalidKind(err error, kind string) bool { - var uerr *errInvalid + var uerr *InvalidError if err == nil || !As(err, &uerr) { return false } diff --git a/pkg/errors/list.go b/pkg/errors/list.go index 349528109..dc7458c41 100644 --- a/pkg/errors/list.go +++ b/pkg/errors/list.go @@ -16,11 +16,15 @@ type ErrorList struct { //nolint: errname // Intentional naming. } func (l *ErrorList) Error() string { - msg := "{" + l.msg - sep := "" - if msg != "" { - sep = ": " + msg := "" + if l.msg != "" { + msg = fmt.Sprintf("%s: ", l.msg) } + + if len(l.errors) == 1 { + return fmt.Sprintf("%s%s", msg, l.errors[0].Error()) + } + sep := "{" for _, e := range l.errors { if e != nil { msg = fmt.Sprintf("%s%s%s", msg, sep, e) @@ -60,6 +64,9 @@ func (l *ErrorList) Result() error { if l == nil || len(l.errors) == 0 { return nil } + if l.msg == "" && len(l.errors) == 1 { + return l.errors[0] + } return l } @@ -72,3 +79,9 @@ func ErrListf(msg string, args ...interface{}) *ErrorList { msg: fmt.Sprintf(msg, args...), } } + +func ErrList(args ...interface{}) *ErrorList { + return &ErrorList{ + msg: fmt.Sprint(args...), + } +} diff --git a/pkg/errors/list_test.go b/pkg/errors/list_test.go new file mode 100644 index 000000000..e3c0b2458 --- /dev/null +++ b/pkg/errors/list_test.go @@ -0,0 +1,73 @@ +// SPDX-FileCopyrightText: 2022 SAP SE or an SAP affiliate company and Open Component Model contributors. +// +// SPDX-License-Identifier: Apache-2.0 + +package errors + +import ( + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("error list", func() { + Context("without message", func() { + It("handles no error", func() { + Expect(ErrListf("").Result()).To(Succeed()) + }) + + It("handles one error", func() { + err := ErrListf("").Add(fmt.Errorf("e1")).Result() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("e1")) + }) + + It("handles two error2", func() { + err := ErrListf("").Add(fmt.Errorf("e1"), fmt.Errorf("e2")).Result() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("{e1, e2}")) + }) + }) + + Context("with message", func() { + It("handles no error", func() { + Expect(ErrListf("msg").Result()).To(Succeed()) + }) + + It("handles one error", func() { + err := ErrListf("msg").Add(fmt.Errorf("e1")).Result() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("msg: e1")) + }) + + It("handles two error2", func() { + err := ErrListf("msg").Add(fmt.Errorf("e1"), fmt.Errorf("e2")).Result() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("msg: {e1, e2}")) + }) + }) + + Context("is a", func() { + It("handles single nested", func() { + err := ErrListf("msg").Add(ErrInvalid()).Result() + Expect(err).To(HaveOccurred()) + Expect(IsA(err, ErrInvalid())).To(BeTrue()) + }) + + It("handles nested single nested", func() { + err := ErrListf("msg").Add(ErrInvalid()).Result() + Expect(err).To(HaveOccurred()) + Expect(IsA(err, ErrInvalid())).To(BeTrue()) + }) + + It("gets single nested", func() { + err := Wrapf(ErrListf("msg").Add(ErrInvalid("test")).Result(), "top") + exp := &InvalidError{} + Expect(err).To(HaveOccurred()) + Expect(As(err, &exp)).To(BeTrue()) + Expect(exp.Error()).To(Equal("\"test\" is invalid")) + }) + }) + +}) diff --git a/pkg/errors/notfound.go b/pkg/errors/notfound.go index 92910eee9..86f942501 100644 --- a/pkg/errors/notfound.go +++ b/pkg/errors/notfound.go @@ -4,26 +4,26 @@ package errors -type errNotFound struct { +type NotFoundError struct { errinfo } var formatNotFound = NewDefaultFormatter("", "not found", "in") func ErrNotFound(spec ...string) error { - return &errNotFound{newErrInfo(formatNotFound, spec...)} + return &NotFoundError{newErrInfo(formatNotFound, spec...)} } func ErrNotFoundWrap(err error, spec ...string) error { - return &errNotFound{wrapErrInfo(err, formatNotFound, spec...)} + return &NotFoundError{wrapErrInfo(err, formatNotFound, spec...)} } func IsErrNotFound(err error) bool { - return IsA(err, &errNotFound{}) + return IsA(err, &NotFoundError{}) } func IsErrNotFoundKind(err error, kind string) bool { - var uerr *errNotFound + var uerr *NotFoundError if err == nil || !As(err, &uerr) { return false } diff --git a/pkg/errors/notimpl.go b/pkg/errors/notimpl.go index 7c820bc07..35d8f71f9 100644 --- a/pkg/errors/notimpl.go +++ b/pkg/errors/notimpl.go @@ -4,22 +4,22 @@ package errors -type errNotImplemented struct { +type NotImplementedError struct { errinfo } var formatNotImplemented = NewDefaultFormatter("", "not implemented", "by") func ErrNotImplemented(spec ...string) error { - return &errNotImplemented{newErrInfo(formatNotImplemented, spec...)} + return &NotImplementedError{newErrInfo(formatNotImplemented, spec...)} } func IsErrNotImplemented(err error) bool { - return IsA(err, &errNotImplemented{}) + return IsA(err, &NotImplementedError{}) } func IsErrNotImplementedKind(err error, kind string) bool { - var uerr *errNotImplemented + var uerr *NotImplementedError if err == nil || !As(err, &uerr) { return false } diff --git a/pkg/errors/notsupported.go b/pkg/errors/notsupported.go index 48e282729..e863abda6 100644 --- a/pkg/errors/notsupported.go +++ b/pkg/errors/notsupported.go @@ -4,22 +4,22 @@ package errors -type errNotSupported struct { +type NotSupportedError struct { errinfo } var formatNotSupported = NewDefaultFormatter("", "not supported", "by") func ErrNotSupported(spec ...string) error { - return &errNotSupported{newErrInfo(formatNotSupported, spec...)} + return &NotSupportedError{newErrInfo(formatNotSupported, spec...)} } func IsErrNotSupported(err error) bool { - return IsA(err, &errNotSupported{}) + return IsA(err, &NotSupportedError{}) } func IsErrNotSupportedKind(err error, kind string) bool { - var uerr *errNotSupported + var uerr *NotSupportedError if err == nil || !As(err, &uerr) { return false } diff --git a/pkg/errors/readonly.go b/pkg/errors/readonly.go index cea803371..477af5bb3 100644 --- a/pkg/errors/readonly.go +++ b/pkg/errors/readonly.go @@ -4,22 +4,22 @@ package errors -type errReadOnly struct { +type ReadOnlyError struct { errinfo } var formatReadOnly = NewDefaultFormatter("is", "readonly", "in") func ErrReadOnly(spec ...string) error { - return &errReadOnly{newErrInfo(formatReadOnly, spec...)} + return &ReadOnlyError{newErrInfo(formatReadOnly, spec...)} } func IsErrReadOnly(err error) bool { - return IsA(err, &errReadOnly{}) + return IsA(err, &ReadOnlyError{}) } func IsErrReadOnlyKind(err error, kind string) bool { - var uerr *errReadOnly + var uerr *ReadOnlyError if err == nil || !As(err, &uerr) { return false } diff --git a/pkg/errors/recursion.go b/pkg/errors/recursion.go index ffb935d84..cb9fb0dc4 100644 --- a/pkg/errors/recursion.go +++ b/pkg/errors/recursion.go @@ -9,7 +9,7 @@ import ( "reflect" ) -type recursionError struct { +type RecursionError struct { wrapped error kind string elem interface{} @@ -18,14 +18,14 @@ type recursionError struct { // ErrRecusion describes a resursion errors caused by a dedicated element with an element history. func ErrRecusion(kind string, elem interface{}, hist interface{}) error { - return &recursionError{nil, kind, elem, ToInterfaceSlice(hist)} + return &RecursionError{nil, kind, elem, ToInterfaceSlice(hist)} } func ErrRecusionWrap(err error, kind string, elem interface{}, hist interface{}) error { - return &recursionError{err, kind, elem, ToInterfaceSlice(hist)} + return &RecursionError{err, kind, elem, ToInterfaceSlice(hist)} } -func (e *recursionError) Error() string { +func (e *RecursionError) Error() string { msg := fmt.Sprintf("%s recursion: use of %v", e.kind, e.elem) if len(e.hist) > 0 { s := "" @@ -42,24 +42,24 @@ func (e *recursionError) Error() string { return msg } -func (e *recursionError) Unwrap() error { +func (e *RecursionError) Unwrap() error { return e.wrapped } -func (e *recursionError) Elem() interface{} { +func (e *RecursionError) Elem() interface{} { return e.elem } -func (e *recursionError) Kind() string { +func (e *RecursionError) Kind() string { return e.kind } func IsErrRecusion(err error) bool { - return IsA(err, &recursionError{}) + return IsA(err, &RecursionError{}) } func IsErrRecursionKind(err error, kind string) bool { - var uerr *recursionError + var uerr *RecursionError if err == nil || !As(err, &uerr) { return false } diff --git a/pkg/errors/unknown.go b/pkg/errors/unknown.go index 56e2589b7..4fcead506 100644 --- a/pkg/errors/unknown.go +++ b/pkg/errors/unknown.go @@ -4,22 +4,22 @@ package errors -type errUnknown struct { +type UnknownError struct { errinfo } var formatUnknown = NewDefaultFormatter("is", "unknown", "for") func ErrUnknown(spec ...string) error { - return &errUnknown{newErrInfo(formatUnknown, spec...)} + return &UnknownError{newErrInfo(formatUnknown, spec...)} } func IsErrUnknown(err error) bool { - return IsA(err, &errUnknown{}) + return IsA(err, &UnknownError{}) } func IsErrUnknownKind(err error, kind string) bool { - var uerr *errUnknown + var uerr *UnknownError if err == nil || !As(err, &uerr) { return false } diff --git a/pkg/utils/finalizer.go b/pkg/utils/finalizer.go index 876a778a1..0fa7ddbd6 100644 --- a/pkg/utils/finalizer.go +++ b/pkg/utils/finalizer.go @@ -12,24 +12,29 @@ import ( ) // Finalizer gathers finalization functions and calls -// them by calling the Finalize method. +// them by calling the Finalize method(s). // Add and Finalize may be called in any sequence and number. // Finalize just calls the aggregated functions between its // last and the actual call. // This way it can be used together with defer to clean up // stuff when leaving a function and combine it with -// controlled intermediate cleanup need, for example as part of +// controlled intermediate cleanup needed, for example as part of // a loop block. type Finalizer struct { lock sync.Mutex pending []func() error + nested *Finalizer } +// Lock locks a given Locker and unlocks it again +// during finalization. func (f *Finalizer) Lock(locker sync.Locker) *Finalizer { locker.Lock() return f.WithVoid(locker.Unlock) } +// WithVoid registers a simple function to be +// called on finalization. func (f *Finalizer) WithVoid(fi func()) *Finalizer { return f.With(func() error { fi(); return nil }) } @@ -44,6 +49,8 @@ func (f *Finalizer) With(fi func() error) *Finalizer { return f } +// Close will finalize the given object by calling +// its Close function when the finalizer is finalized. func (f *Finalizer) Close(c io.Closer) *Finalizer { if c != nil { f.With(c.Close) @@ -51,24 +58,71 @@ func (f *Finalizer) Close(c io.Closer) *Finalizer { return f } -func (f *Finalizer) Include(c *Finalizer) *Finalizer { - if c != nil { - f.With(c.Finalize) +// Include includes the finalization of a given +// finalizer. +func (f *Finalizer) Include(fi *Finalizer) *Finalizer { + if fi != nil { + f.With(fi.Finalize) } return f } +// New return a new finalizer included in the actual one. +func (f *Finalizer) New() *Finalizer { + n := &Finalizer{} + f.Include(n) + return n +} + +// Nested returns a linked finalizer usable in a nested block, +// which can be separately finalized. It is intended for sequential +// use, for example in a for loop. Successive calls +// will provide the same finalizer. The nested finalizer +// SHOULD be finalized at the end of its scope before +// it is requested, again, for the next nested usage. +func (f *Finalizer) Nested() *Finalizer { + f.lock.Lock() + defer f.lock.Unlock() + + if f.nested == nil { + f.nested = &Finalizer{} + f.pending = append(f.pending, f.nested.Finalize) + } + return f.nested +} + func (f *Finalizer) Length() int { f.lock.Lock() defer f.lock.Unlock() return len(f.pending) } +// FinalizeWithErrorPropagation calls all finalizations in the reverse order of +// their registration and propagates a potential error to the given error +// variable incorporating an already existing error. +// This is especially intended to be used in a deferred mode to adapt +// the error code of a function to incorporate finalization errors. +func (f *Finalizer) FinalizeWithErrorPropagation(efferr *error) { + errors.PropagateError(efferr, f.Finalize) +} + +// FinalizeWithErrorPropagationf calls all finalizations in the reverse order of +// their registration and propagates a potential error to the given error +// variable incorporating an already existing error. +// This is especially intended to be used in a deferred mode to adapt +// the error code of a function to incorporate finalization errors. +// The final error will be wrapped by the given common context. +func (f *Finalizer) FinalizeWithErrorPropagationf(efferr *error, msg string, args ...interface{}) { + errors.PropagateErrorf(efferr, f.Finalize, msg, args...) +} + +// Finalize calls all finalizations in the reverse order of +// their registration. func (f *Finalizer) Finalize() error { f.lock.Lock() defer f.lock.Unlock() - list := errors.ErrListf("finalize") + list := errors.ErrList() l := len(f.pending) for i := range f.pending { list.Add(f.pending[l-i-1]()) diff --git a/pkg/utils/finalizer_test.go b/pkg/utils/finalizer_test.go index 4cb9e3cc2..dc174a2d8 100644 --- a/pkg/utils/finalizer_test.go +++ b/pkg/utils/finalizer_test.go @@ -5,6 +5,8 @@ package utils_test import ( + "fmt" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -52,4 +54,129 @@ var _ = Describe("finalizer", func() { Expect(order).To(Equal(Order{"D", "C"})) Expect(finalize.Length()).To(Equal(0)) }) + + It("separately finalizes a Nested finalizer", func() { + var finalize utils.Finalizer + var order Order + + finalize.With(F("A", &order)) + finalize.With(F("B", &order)) + + { + finalize := finalize.Nested() + finalize.With(F("C", &order)) + finalize.Finalize() + Expect(order).To(Equal(Order{"C"})) + } + + { + finalize := finalize.Nested() + finalize.With(F("D", &order)) + finalize.Finalize() + Expect(order).To(Equal(Order{"C", "D"})) + } + + { + finalize := finalize.Nested() + finalize.With(F("E", &order)) + } + + finalize.Finalize() + Expect(order).To(Equal(Order{"C", "D", "E", "B", "A"})) + Expect(finalize.Length()).To(Equal(0)) + }) + + It("separately finalizes new finalizers", func() { + var finalize utils.Finalizer + var order Order + + finalize.With(F("A", &order)) + finalize.With(F("B", &order)) + + { + finalize := finalize.New() + finalize.With(F("C", &order)) + } + + { + finalize := finalize.Nested() + finalize.With(F("D", &order)) + finalize.Finalize() + Expect(order).To(Equal(Order{"D"})) + } + + { + finalize := finalize.New() + finalize.With(F("E", &order)) + } + + finalize.Finalize() + Expect(order).To(Equal(Order{"D", "E", "C", "B", "A"})) + Expect(finalize.Length()).To(Equal(0)) + }) + + Context("with error propagation", func() { + Context("without context", func() { + It("succeeds", func() { + Expect(testFunc("", nil, true)).To(Succeed()) + }) + + It("fails ", func() { + err := testFunc("", fmt.Errorf("failed"), true) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("failed")) + }) + + It("succeeds with failing finalizer", func() { + err := testFunc("", nil, false) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("error occurred")) + }) + + It("fails with failing finalizer", func() { + err := testFunc("", fmt.Errorf("failed"), false) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("{failed, error occurred}")) + }) + }) + + Context("with context", func() { + It("succeeds", func() { + Expect(testFunc("context", nil, true)).To(Succeed()) + }) + + It("fails ", func() { + err := testFunc("context", fmt.Errorf("failed"), true) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("context: failed")) + }) + + It("succeeds with failing finalizer", func() { + err := testFunc("context", nil, false) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("context: error occurred")) + }) + + It("fails with failing finalizer", func() { + err := testFunc("context", fmt.Errorf("failed"), false) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("context: {failed, error occurred}")) + }) + }) + }) }) + +func errfunc(succeed bool) func() error { + if succeed { + return func() error { return nil } + } + return func() error { return fmt.Errorf("error occurred") } +} + +func testFunc(msg string, err error, succeed bool) (efferr error) { + var finalize utils.Finalizer + + defer finalize.FinalizeWithErrorPropagationf(&efferr, msg) + finalize.With(errfunc(succeed)) + return err +} diff --git a/pkg/utils/utils_suite_test.go b/pkg/utils/suite_test.go similarity index 100% rename from pkg/utils/utils_suite_test.go rename to pkg/utils/suite_test.go