From 77f189266472666c76601b88a2a77d75fd3df1a1 Mon Sep 17 00:00:00 2001 From: Josh Michielsen Date: Tue, 21 Feb 2023 14:00:26 +0000 Subject: [PATCH 01/10] add panic package and handler code --- pkg/utils/panics/panics.go | 55 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 pkg/utils/panics/panics.go diff --git a/pkg/utils/panics/panics.go b/pkg/utils/panics/panics.go new file mode 100644 index 0000000000..5a97d5a1bf --- /dev/null +++ b/pkg/utils/panics/panics.go @@ -0,0 +1,55 @@ +// SPDX-FileCopyrightText: 2022 SAP SE or an SAP affiliate company and Open Component Model contributors. +// +// SPDX-License-Identifier: Apache-2.0 + +package panics + +import ( + "fmt" + "runtime" + + "github.com/open-component-model/ocm/pkg/logging" +) + +type PanicHandler func(interface{}) bool + +var ( + PanicHandlers = []PanicHandler{logHandler} +) + +func HandlePanic(additionalHandlers ...PanicHandler) { + reallyCrash := true + + if r := recover(); r != nil { + for _, fn := range PanicHandlers { + reallyCrash = fn(r) + } + + for _, fn := range additionalHandlers { + reallyCrash = fn(r) + } + + if reallyCrash { + panic(r) + } + } +} + +func RegisterPanicHandler(handler PanicHandler) { + PanicHandlers = append(PanicHandlers, handler) +} + +func logHandler(r interface{}) bool { + // Same as stdlib http server code. Manually allocate stack trace buffer size + // to prevent excessively large logs + const size = 64 << 10 + stacktrace := make([]byte, size) + stacktrace = stacktrace[:runtime.Stack(stacktrace, false)] + if _, ok := r.(string); ok { + logging.Logger().Error(fmt.Sprintf("Observed a panic: %#v\n%s", r, stacktrace)) + } else { + logging.Logger().Error(fmt.Sprintf("Observed a panic: %#v (%v)\n%s", r, r, stacktrace)) + } + + return true +} From 50d1b180091ea35c6ba98f52784b18c5c8fd3aa0 Mon Sep 17 00:00:00 2001 From: Josh Michielsen Date: Thu, 23 Feb 2023 15:16:33 +0000 Subject: [PATCH 02/10] add panic handler to panic call sites --- cmds/ocm/commands/common/options/closureoption/option.go | 2 ++ cmds/ocm/pkg/data/dll.go | 3 +++ cmds/ocm/pkg/processing/buffer.go | 3 +++ cmds/ocm/pkg/processing/pool.go | 3 +++ cmds/ocm/pkg/utils/handling.go | 2 ++ pkg/cobrautils/flag/string_to_string.go | 2 ++ pkg/cobrautils/links.go | 2 ++ pkg/common/accessobj/accessstate.go | 2 ++ pkg/contexts/datacontext/util.go | 3 +++ pkg/contexts/oci/attrs/cacheattr/attr.go | 2 ++ pkg/contexts/oci/cpi/flavor_artifact.go | 2 ++ pkg/contexts/oci/cpi/flavor_index.go | 2 ++ pkg/contexts/oci/cpi/flavor_manifest.go | 2 ++ pkg/contexts/oci/cpi/support/flavor_artifact.go | 2 ++ pkg/contexts/oci/repositories/ocireg/blobs.go | 2 ++ .../versions/ocm.software/v3alpha1/jsonscheme/jsonscheme.go | 2 ++ pkg/contexts/ocm/compdesc/versions/v2/jsonscheme/jsonscheme.go | 2 ++ pkg/contexts/ocm/internal/builder.go | 2 ++ pkg/contexts/ocm/internal/builtin.go | 2 ++ pkg/contexts/ocm/internal/digesthandler.go | 2 ++ pkg/contexts/ocm/plugin/ppi/utils.go | 2 ++ pkg/contexts/ocm/utils.go | 2 ++ pkg/regex/regex.go | 3 +++ pkg/runtime/scheme.go | 3 +++ pkg/runtime/utils.go | 2 ++ pkg/runtime/versionedtype.go | 3 +++ pkg/signing/handlers/rsa/format.go | 2 ++ pkg/signing/normalization.go | 2 ++ pkg/utils/utils.go | 2 ++ 29 files changed, 65 insertions(+) diff --git a/cmds/ocm/commands/common/options/closureoption/option.go b/cmds/ocm/commands/common/options/closureoption/option.go index b1a47b7c71..5c5f03ad42 100644 --- a/cmds/ocm/commands/common/options/closureoption/option.go +++ b/cmds/ocm/commands/common/options/closureoption/option.go @@ -16,6 +16,7 @@ import ( "github.com/open-component-model/ocm/pkg/contexts/ocm/transfer/transferhandler" "github.com/open-component-model/ocm/pkg/contexts/ocm/transfer/transferhandler/standard" "github.com/open-component-model/ocm/pkg/utils" + "github.com/open-component-model/ocm/pkg/utils/panics" ) func From(o options.OptionSetProvider) *Option { @@ -34,6 +35,7 @@ type Option struct { } func New(elemname string, settings ...interface{}) *Option { + defer panics.HandlePanic() o := &Option{ElementName: elemname, AddReferencePath: options.Always()} for _, s := range settings { switch v := s.(type) { diff --git a/cmds/ocm/pkg/data/dll.go b/cmds/ocm/pkg/data/dll.go index f1b61d6078..770e9e7862 100644 --- a/cmds/ocm/pkg/data/dll.go +++ b/cmds/ocm/pkg/data/dll.go @@ -6,6 +6,8 @@ package data import ( "sync" + + "github.com/open-component-model/ocm/pkg/utils/panics" ) type DLLRoot struct { @@ -78,6 +80,7 @@ func (this *DLL) Set(p interface{}) { } func (this *DLL) Append(d *DLL) { + defer panics.HandlePanic() if d.next != nil || d.prev != nil { panic("dll element already in use") } diff --git a/cmds/ocm/pkg/processing/buffer.go b/cmds/ocm/pkg/processing/buffer.go index 7fe2d9e810..88d91b1318 100644 --- a/cmds/ocm/pkg/processing/buffer.go +++ b/cmds/ocm/pkg/processing/buffer.go @@ -12,6 +12,7 @@ import ( "github.com/mandelsoft/logging" "github.com/open-component-model/ocm/cmds/ocm/pkg/data" + "github.com/open-component-model/ocm/pkg/utils/panics" ) type Index = IndexArray @@ -72,6 +73,7 @@ type ProcessingEntry struct { type SubEntries int func NewEntry(i Index, v interface{}, opts ...interface{}) ProcessingEntry { + defer panics.HandlePanic() max := -1 sub := 0 valid := true @@ -365,6 +367,7 @@ func (ob *orderedBuffer) SetFrame(frame BufferFrame) { } func (ob *orderedBuffer) Add(e ProcessingEntry) bool { + defer panics.HandlePanic() e.Index.Validate(e.MaxIndex) ob.simple.Add(e) n := data.NewDLL(&e) diff --git a/cmds/ocm/pkg/processing/pool.go b/cmds/ocm/pkg/processing/pool.go index 5db6742ce9..6a8bf89753 100644 --- a/cmds/ocm/pkg/processing/pool.go +++ b/cmds/ocm/pkg/processing/pool.go @@ -6,6 +6,8 @@ package processing import ( "sync" + + "github.com/open-component-model/ocm/pkg/utils/panics" ) type ProcessorPool interface { @@ -55,6 +57,7 @@ func (this *_ProcessorPool) Request() { } func (this *_ProcessorPool) Exec(f func()) { + defer panics.HandlePanic() this.lock.Lock() defer this.lock.Unlock() if this.uses == 0 { diff --git a/cmds/ocm/pkg/utils/handling.go b/cmds/ocm/pkg/utils/handling.go index 1d77ce311b..7802f7b1bb 100644 --- a/cmds/ocm/pkg/utils/handling.go +++ b/cmds/ocm/pkg/utils/handling.go @@ -11,6 +11,7 @@ import ( "github.com/open-component-model/ocm/cmds/ocm/pkg/output" "github.com/open-component-model/ocm/pkg/errors" + "github.com/open-component-model/ocm/pkg/utils/panics" ) type ElemSpec interface { @@ -45,6 +46,7 @@ func StringElemSpecs(args ...string) []ElemSpec { } func ElemSpecs(list interface{}) []ElemSpec { + defer panics.HandlePanic() if list == nil { return nil } diff --git a/pkg/cobrautils/flag/string_to_string.go b/pkg/cobrautils/flag/string_to_string.go index c20948070b..5a3139e53a 100644 --- a/pkg/cobrautils/flag/string_to_string.go +++ b/pkg/cobrautils/flag/string_to_string.go @@ -13,6 +13,7 @@ import ( "fmt" "strings" + "github.com/open-component-model/ocm/pkg/utils/panics" "github.com/spf13/pflag" ) @@ -73,6 +74,7 @@ func (s *stringToStringValue[T]) Type() string { } func (s *stringToStringValue[T]) String() string { + defer panics.HandlePanic() records := make([]string, 0, len(*s.value)>>1) for k, v := range *s.value { records = append(records, k+"="+v) diff --git a/pkg/cobrautils/links.go b/pkg/cobrautils/links.go index 2257e9dc6f..78931153a3 100644 --- a/pkg/cobrautils/links.go +++ b/pkg/cobrautils/links.go @@ -8,6 +8,7 @@ import ( "fmt" "strings" + "github.com/open-component-model/ocm/pkg/utils/panics" "github.com/spf13/cobra" ) @@ -32,6 +33,7 @@ func FormatLinkWithHandler(linkhandler func(string) string) func(string) string } func SubstituteCommandLinks(desc string, linkformat func(string) string) ([]string, string) { + defer panics.HandlePanic() var links []string for { link := strings.Index(desc, "") diff --git a/pkg/common/accessobj/accessstate.go b/pkg/common/accessobj/accessstate.go index 81b4ddbf04..90ffef7a69 100644 --- a/pkg/common/accessobj/accessstate.go +++ b/pkg/common/accessobj/accessstate.go @@ -14,6 +14,7 @@ import ( "github.com/open-component-model/ocm/pkg/common/accessio" "github.com/open-component-model/ocm/pkg/errors" + "github.com/open-component-model/ocm/pkg/utils/panics" ) // These objects deal with descriptor based state descriptions @@ -221,6 +222,7 @@ func (s *state) Refresh() error { } func (s *state) GetOriginalState() interface{} { + defer panics.HandlePanic() if s.originalBlob == nil { return nil } diff --git a/pkg/contexts/datacontext/util.go b/pkg/contexts/datacontext/util.go index 035c720015..c9ef282c39 100644 --- a/pkg/contexts/datacontext/util.go +++ b/pkg/contexts/datacontext/util.go @@ -7,6 +7,8 @@ package datacontext import ( "context" "fmt" + + "github.com/open-component-model/ocm/pkg/utils/panics" ) // ForContextByKey retrieves the context for a given key to be used for a context.Context. @@ -27,6 +29,7 @@ type ElementCopyable[T any] interface { type ElementCreator[T any] func(base ...T) T func SetupElement[T ElementCopyable[T]](mode BuilderMode, target *T, create ElementCreator[T], def T) { + defer panics.HandlePanic() var zero T if *target == zero { switch mode { diff --git a/pkg/contexts/oci/attrs/cacheattr/attr.go b/pkg/contexts/oci/attrs/cacheattr/attr.go index 6935a110fc..b6917ccc9f 100644 --- a/pkg/contexts/oci/attrs/cacheattr/attr.go +++ b/pkg/contexts/oci/attrs/cacheattr/attr.go @@ -13,6 +13,7 @@ import ( "github.com/open-component-model/ocm/pkg/contexts/datacontext" "github.com/open-component-model/ocm/pkg/errors" "github.com/open-component-model/ocm/pkg/runtime" + "github.com/open-component-model/ocm/pkg/utils/panics" ) const ( @@ -45,6 +46,7 @@ func (a AttributeType) Encode(v interface{}, marshaller runtime.Marshaler) ([]by } func (a AttributeType) Decode(data []byte, unmarshaller runtime.Unmarshaler) (interface{}, error) { + defer panics.HandlePanic() var value string err := unmarshaller.Unmarshal(data, &value) if value != "" { diff --git a/pkg/contexts/oci/cpi/flavor_artifact.go b/pkg/contexts/oci/cpi/flavor_artifact.go index 4bce489cee..92f44fa487 100644 --- a/pkg/contexts/oci/cpi/flavor_artifact.go +++ b/pkg/contexts/oci/cpi/flavor_artifact.go @@ -14,6 +14,7 @@ import ( "github.com/open-component-model/ocm/pkg/contexts/oci/artdesc" "github.com/open-component-model/ocm/pkg/contexts/oci/internal" "github.com/open-component-model/ocm/pkg/errors" + "github.com/open-component-model/ocm/pkg/utils/panics" ) var ErrNoIndex = errors.New("manifest does not support access to subsequent artifacts") @@ -67,6 +68,7 @@ func NewArtifactForBlob(access ArtifactSetContainer, blob accessio.BlobAccess) ( } func NewArtifact(access ArtifactSetContainer, defs ...*artdesc.Artifact) (ArtifactAccess, error) { + defer panics.HandlePanic() var def *artdesc.Artifact if len(defs) != 0 && defs[0] != nil { def = defs[0] diff --git a/pkg/contexts/oci/cpi/flavor_index.go b/pkg/contexts/oci/cpi/flavor_index.go index ba24b82213..84bf4303b9 100644 --- a/pkg/contexts/oci/cpi/flavor_index.go +++ b/pkg/contexts/oci/cpi/flavor_index.go @@ -12,6 +12,7 @@ import ( "github.com/open-component-model/ocm/pkg/contexts/oci/artdesc" "github.com/open-component-model/ocm/pkg/contexts/oci/internal" "github.com/open-component-model/ocm/pkg/errors" + "github.com/open-component-model/ocm/pkg/utils/panics" ) type IndexImpl struct { @@ -21,6 +22,7 @@ type IndexImpl struct { var _ IndexAccess = (*IndexImpl)(nil) func NewIndex(access ArtifactSetContainer, defs ...*artdesc.Index) (internal.IndexAccess, error) { + defer panics.HandlePanic() var def *artdesc.Index if len(defs) != 0 && defs[0] != nil { def = defs[0] diff --git a/pkg/contexts/oci/cpi/flavor_manifest.go b/pkg/contexts/oci/cpi/flavor_manifest.go index 37791e80d6..ed63902e91 100644 --- a/pkg/contexts/oci/cpi/flavor_manifest.go +++ b/pkg/contexts/oci/cpi/flavor_manifest.go @@ -13,6 +13,7 @@ import ( "github.com/open-component-model/ocm/pkg/common/accessobj" "github.com/open-component-model/ocm/pkg/contexts/oci/artdesc" "github.com/open-component-model/ocm/pkg/errors" + "github.com/open-component-model/ocm/pkg/utils/panics" ) type ManifestImpl struct { @@ -22,6 +23,7 @@ type ManifestImpl struct { var _ ManifestAccess = (*ManifestImpl)(nil) func NewManifest(access ArtifactSetContainer, defs ...*artdesc.Manifest) (*ManifestImpl, error) { + defer panics.HandlePanic() var def *artdesc.Manifest if len(defs) != 0 && defs[0] != nil { def = defs[0] diff --git a/pkg/contexts/oci/cpi/support/flavor_artifact.go b/pkg/contexts/oci/cpi/support/flavor_artifact.go index 918796b6f7..8410eae83f 100644 --- a/pkg/contexts/oci/cpi/support/flavor_artifact.go +++ b/pkg/contexts/oci/cpi/support/flavor_artifact.go @@ -15,6 +15,7 @@ import ( "github.com/open-component-model/ocm/pkg/contexts/oci/cpi" "github.com/open-component-model/ocm/pkg/contexts/oci/internal" "github.com/open-component-model/ocm/pkg/errors" + "github.com/open-component-model/ocm/pkg/utils/panics" ) var ErrNoIndex = errors.New("manifest does not support access to subsequent artifacts") @@ -39,6 +40,7 @@ func NewArtifactForBlob(container ArtifactSetContainerImpl, blob accessio.BlobAc } func NewArtifact(container ArtifactSetContainerImpl, defs ...*artdesc.Artifact) (cpi.ArtifactAccess, error) { + defer panics.HandlePanic() var def *artdesc.Artifact if len(defs) != 0 && defs[0] != nil { def = defs[0] diff --git a/pkg/contexts/oci/repositories/ocireg/blobs.go b/pkg/contexts/oci/repositories/ocireg/blobs.go index 6d9ae53df1..4368211398 100644 --- a/pkg/contexts/oci/repositories/ocireg/blobs.go +++ b/pkg/contexts/oci/repositories/ocireg/blobs.go @@ -16,6 +16,7 @@ import ( "github.com/open-component-model/ocm/pkg/contexts/oci/cpi" "github.com/open-component-model/ocm/pkg/docker/resolve" "github.com/open-component-model/ocm/pkg/errors" + "github.com/open-component-model/ocm/pkg/utils/panics" ) type BlobContainer interface { @@ -79,6 +80,7 @@ func newBlobContainer(mime string, fetcher resolve.Fetcher, pusher resolve.Pushe } func NewBlobContainer(cache accessio.BlobCache, mime string, fetcher resolve.Fetcher, pusher resolve.Pusher) BlobContainer { + defer panics.HandlePanic() c := newBlobContainer(mime, fetcher, pusher) if cache == nil { diff --git a/pkg/contexts/ocm/compdesc/versions/ocm.software/v3alpha1/jsonscheme/jsonscheme.go b/pkg/contexts/ocm/compdesc/versions/ocm.software/v3alpha1/jsonscheme/jsonscheme.go index db3bdbfcaa..ecc8f322ce 100644 --- a/pkg/contexts/ocm/compdesc/versions/ocm.software/v3alpha1/jsonscheme/jsonscheme.go +++ b/pkg/contexts/ocm/compdesc/versions/ocm.software/v3alpha1/jsonscheme/jsonscheme.go @@ -12,12 +12,14 @@ import ( "fmt" "github.com/ghodss/yaml" + "github.com/open-component-model/ocm/pkg/utils/panics" "github.com/xeipuuv/gojsonschema" ) var Schema *gojsonschema.Schema func init() { + defer panics.HandlePanic() dataBytes, err := ResourcesComponentDescriptorOcmV3SchemaYamlBytes() if err != nil { panic(err) diff --git a/pkg/contexts/ocm/compdesc/versions/v2/jsonscheme/jsonscheme.go b/pkg/contexts/ocm/compdesc/versions/v2/jsonscheme/jsonscheme.go index 95e3fdc970..c48b93fe98 100644 --- a/pkg/contexts/ocm/compdesc/versions/v2/jsonscheme/jsonscheme.go +++ b/pkg/contexts/ocm/compdesc/versions/v2/jsonscheme/jsonscheme.go @@ -12,12 +12,14 @@ import ( "fmt" "github.com/ghodss/yaml" + "github.com/open-component-model/ocm/pkg/utils/panics" "github.com/xeipuuv/gojsonschema" ) var Schema *gojsonschema.Schema func init() { + defer panics.HandlePanic() dataBytes, err := ResourcesComponentDescriptorV2SchemaYamlBytes() if err != nil { panic(err) diff --git a/pkg/contexts/ocm/internal/builder.go b/pkg/contexts/ocm/internal/builder.go index 0b7489aec0..af132245e6 100644 --- a/pkg/contexts/ocm/internal/builder.go +++ b/pkg/contexts/ocm/internal/builder.go @@ -13,6 +13,7 @@ import ( "github.com/open-component-model/ocm/pkg/contexts/datacontext" "github.com/open-component-model/ocm/pkg/contexts/oci" "github.com/open-component-model/ocm/pkg/runtime" + "github.com/open-component-model/ocm/pkg/utils/panics" ) type Builder struct { @@ -183,6 +184,7 @@ type delegatingDecoder struct { var _ runtime.TypedObjectDecoder = (*delegatingDecoder)(nil) func (d *delegatingDecoder) Decode(data []byte, unmarshaler runtime.Unmarshaler) (runtime.TypedObject, error) { + defer panics.HandlePanic() d.lock.Lock() defer d.lock.Unlock() diff --git a/pkg/contexts/ocm/internal/builtin.go b/pkg/contexts/ocm/internal/builtin.go index fb0ef94e65..69618bf16f 100644 --- a/pkg/contexts/ocm/internal/builtin.go +++ b/pkg/contexts/ocm/internal/builtin.go @@ -6,6 +6,7 @@ package internal import ( "github.com/open-component-model/ocm/pkg/contexts/oci" + "github.com/open-component-model/ocm/pkg/utils/panics" ) // @@ -16,6 +17,7 @@ type OCISpecFunction func(ctx oci.Context) (RepositoryType, error) var ociimpl OCISpecFunction func RegisterOCIImplementation(impl OCISpecFunction) { + defer panics.HandlePanic() if ociimpl != nil { panic("oci implementation already registered") } diff --git a/pkg/contexts/ocm/internal/digesthandler.go b/pkg/contexts/ocm/internal/digesthandler.go index b41f2c6d81..6d563189ba 100644 --- a/pkg/contexts/ocm/internal/digesthandler.go +++ b/pkg/contexts/ocm/internal/digesthandler.go @@ -12,6 +12,7 @@ import ( metav1 "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/meta/v1" "github.com/open-component-model/ocm/pkg/signing" "github.com/open-component-model/ocm/pkg/utils" + "github.com/open-component-model/ocm/pkg/utils/panics" ) type DigesterType struct { @@ -259,6 +260,7 @@ func (r *blobDigesterRegistry) DetermineDigests(restype string, preferred signin } func MustRegisterDigester(digester BlobDigester, arttypes ...string) { + defer panics.HandlePanic() err := DefaultBlobDigesterRegistry.Register(digester, arttypes...) if err != nil { panic(err) diff --git a/pkg/contexts/ocm/plugin/ppi/utils.go b/pkg/contexts/ocm/plugin/ppi/utils.go index e41407a67e..671a9ad167 100644 --- a/pkg/contexts/ocm/plugin/ppi/utils.go +++ b/pkg/contexts/ocm/plugin/ppi/utils.go @@ -6,6 +6,7 @@ package ppi import ( "github.com/open-component-model/ocm/pkg/runtime" + "github.com/open-component-model/ocm/pkg/utils/panics" ) type decoder runtime.TypedObjectDecoder @@ -19,6 +20,7 @@ type AccessMethodBase struct { } func MustNewAccessMethodBase(name, version string, proto AccessSpec, desc string, format string) AccessMethodBase { + defer panics.HandlePanic() decoder, err := runtime.NewDirectDecoder(proto) if err != nil { panic(err) diff --git a/pkg/contexts/ocm/utils.go b/pkg/contexts/ocm/utils.go index c9e319bf53..37735943b4 100644 --- a/pkg/contexts/ocm/utils.go +++ b/pkg/contexts/ocm/utils.go @@ -15,11 +15,13 @@ import ( "github.com/open-component-model/ocm/pkg/contexts/ocm/cpi" "github.com/open-component-model/ocm/pkg/contexts/ocm/repositories/ctf" "github.com/open-component-model/ocm/pkg/errors" + "github.com/open-component-model/ocm/pkg/utils/panics" ) //////////////////////////////////////////////////////////////////////////////// func AssureTargetRepository(session Session, ctx Context, targetref string, opts ...interface{}) (Repository, error) { + defer panics.HandlePanic() var format accessio.FileFormat var archive string var fs vfs.FileSystem diff --git a/pkg/regex/regex.go b/pkg/regex/regex.go index 1e4bf0b9e0..1aab10fbc6 100644 --- a/pkg/regex/regex.go +++ b/pkg/regex/regex.go @@ -6,6 +6,8 @@ package regex import ( "regexp" + + "github.com/open-component-model/ocm/pkg/utils/panics" ) var ( @@ -27,6 +29,7 @@ var Match = regexp.MustCompile // Literal compiles s into a literal regular expression, escaping any regexp // reserved characters. func Literal(s string) *regexp.Regexp { + defer panics.HandlePanic() re := Match(regexp.QuoteMeta(s)) if _, complete := re.LiteralPrefix(); !complete { diff --git a/pkg/runtime/scheme.go b/pkg/runtime/scheme.go index 80f89353de..b04d7abb46 100644 --- a/pkg/runtime/scheme.go +++ b/pkg/runtime/scheme.go @@ -13,6 +13,7 @@ import ( "github.com/open-component-model/ocm/pkg/errors" "github.com/open-component-model/ocm/pkg/utils" + "github.com/open-component-model/ocm/pkg/utils/panics" ) // TypeGetter is the interface to be implemented for extracting a type. @@ -53,6 +54,7 @@ type DirectDecoder struct { var _ TypedObjectDecoder = &DirectDecoder{} func MustNewDirectDecoder(proto interface{}) *DirectDecoder { + defer panics.HandlePanic() d, err := NewDirectDecoder(proto) if err != nil { panic(err) @@ -107,6 +109,7 @@ type ConvertingDecoder struct { var _ TypedObjectDecoder = &ConvertingDecoder{} func MustNewConvertingDecoder(proto interface{}, conv TypedObjectConverter) *ConvertingDecoder { + defer panics.HandlePanic() d, err := NewConvertingDecoder(proto, conv) if err != nil { panic(err) diff --git a/pkg/runtime/utils.go b/pkg/runtime/utils.go index f8c1d0d27b..e05e841bc9 100644 --- a/pkg/runtime/utils.go +++ b/pkg/runtime/utils.go @@ -10,9 +10,11 @@ import ( "strings" "github.com/open-component-model/ocm/pkg/errors" + "github.com/open-component-model/ocm/pkg/utils/panics" ) func MustProtoType(proto interface{}) reflect.Type { + defer panics.HandlePanic() t, err := ProtoType(proto) if err != nil { panic(err.Error()) diff --git a/pkg/runtime/versionedtype.go b/pkg/runtime/versionedtype.go index ac3aa01876..bd0c77d666 100644 --- a/pkg/runtime/versionedtype.go +++ b/pkg/runtime/versionedtype.go @@ -6,6 +6,8 @@ package runtime import ( "strings" + + "github.com/open-component-model/ocm/pkg/utils/panics" ) const VersionSeparator = "/" @@ -17,6 +19,7 @@ type VersionedTypedObject interface { } func TypeName(args ...string) string { + defer panics.HandlePanic() if len(args) == 1 { return args[0] } diff --git a/pkg/signing/handlers/rsa/format.go b/pkg/signing/handlers/rsa/format.go index dfcc4d9725..e3b77ead47 100644 --- a/pkg/signing/handlers/rsa/format.go +++ b/pkg/signing/handlers/rsa/format.go @@ -13,6 +13,7 @@ import ( "io" "github.com/open-component-model/ocm/pkg/utils" + "github.com/open-component-model/ocm/pkg/utils/panics" ) func GetPublicKey(key interface{}) (*rsa.PublicKey, []string, error) { @@ -67,6 +68,7 @@ func KeyData(key interface{}) ([]byte, error) { } func PemBlockForKey(priv interface{}, gen ...bool) *pem.Block { + defer panics.HandlePanic() switch k := priv.(type) { case *rsa.PublicKey: if utils.Optional(gen...) { diff --git a/pkg/signing/normalization.go b/pkg/signing/normalization.go index 59509c1a8f..32fa107065 100644 --- a/pkg/signing/normalization.go +++ b/pkg/signing/normalization.go @@ -13,6 +13,7 @@ import ( "github.com/open-component-model/ocm/pkg/errors" "github.com/open-component-model/ocm/pkg/utils" + "github.com/open-component-model/ocm/pkg/utils/panics" ) type Entries []Entry @@ -42,6 +43,7 @@ func (l Entries) ToString(gap string) string { } func toString(v interface{}, gap string) string { + defer panics.HandlePanic() switch castIn := v.(type) { case Entries: return castIn.ToString(gap) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 1a6bd8f203..e8ad34f5a8 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -28,6 +28,7 @@ import ( "sigs.k8s.io/yaml" ocmlog "github.com/open-component-model/ocm/pkg/logging" + "github.com/open-component-model/ocm/pkg/utils/panics" ) // PrintPrettyYaml prints the given objects as yaml if enabled. @@ -296,6 +297,7 @@ func GetOptionFlag(list ...bool) bool { // Must expects a result to be provided without error. func Must[T any](o T, err error) T { + defer panics.HandlePanic() if err != nil { panic(fmt.Errorf("expected a %T, but got error %w", o, err)) } From 97ebc222dba3b597a97998dadd8879f436d2b797 Mon Sep 17 00:00:00 2001 From: Josh Michielsen Date: Thu, 23 Feb 2023 15:22:54 +0000 Subject: [PATCH 03/10] run make format --- pkg/cobrautils/flag/string_to_string.go | 3 ++- pkg/cobrautils/links.go | 3 ++- .../versions/ocm.software/v3alpha1/jsonscheme/jsonscheme.go | 3 ++- .../ocm/compdesc/versions/v2/jsonscheme/jsonscheme.go | 3 ++- pkg/utils/panics/panics.go | 4 +--- 5 files changed, 9 insertions(+), 7 deletions(-) diff --git a/pkg/cobrautils/flag/string_to_string.go b/pkg/cobrautils/flag/string_to_string.go index 5a3139e53a..acf8757bc6 100644 --- a/pkg/cobrautils/flag/string_to_string.go +++ b/pkg/cobrautils/flag/string_to_string.go @@ -13,8 +13,9 @@ import ( "fmt" "strings" - "github.com/open-component-model/ocm/pkg/utils/panics" "github.com/spf13/pflag" + + "github.com/open-component-model/ocm/pkg/utils/panics" ) type stringToStringValue[T ~map[string]string] struct { diff --git a/pkg/cobrautils/links.go b/pkg/cobrautils/links.go index 78931153a3..bc56c0d324 100644 --- a/pkg/cobrautils/links.go +++ b/pkg/cobrautils/links.go @@ -8,8 +8,9 @@ import ( "fmt" "strings" - "github.com/open-component-model/ocm/pkg/utils/panics" "github.com/spf13/cobra" + + "github.com/open-component-model/ocm/pkg/utils/panics" ) func LinkForCmd(cmd *cobra.Command) string { diff --git a/pkg/contexts/ocm/compdesc/versions/ocm.software/v3alpha1/jsonscheme/jsonscheme.go b/pkg/contexts/ocm/compdesc/versions/ocm.software/v3alpha1/jsonscheme/jsonscheme.go index ecc8f322ce..b19aba6d97 100644 --- a/pkg/contexts/ocm/compdesc/versions/ocm.software/v3alpha1/jsonscheme/jsonscheme.go +++ b/pkg/contexts/ocm/compdesc/versions/ocm.software/v3alpha1/jsonscheme/jsonscheme.go @@ -12,8 +12,9 @@ import ( "fmt" "github.com/ghodss/yaml" - "github.com/open-component-model/ocm/pkg/utils/panics" "github.com/xeipuuv/gojsonschema" + + "github.com/open-component-model/ocm/pkg/utils/panics" ) var Schema *gojsonschema.Schema diff --git a/pkg/contexts/ocm/compdesc/versions/v2/jsonscheme/jsonscheme.go b/pkg/contexts/ocm/compdesc/versions/v2/jsonscheme/jsonscheme.go index c48b93fe98..b662cdddb9 100644 --- a/pkg/contexts/ocm/compdesc/versions/v2/jsonscheme/jsonscheme.go +++ b/pkg/contexts/ocm/compdesc/versions/v2/jsonscheme/jsonscheme.go @@ -12,8 +12,9 @@ import ( "fmt" "github.com/ghodss/yaml" - "github.com/open-component-model/ocm/pkg/utils/panics" "github.com/xeipuuv/gojsonschema" + + "github.com/open-component-model/ocm/pkg/utils/panics" ) var Schema *gojsonschema.Schema diff --git a/pkg/utils/panics/panics.go b/pkg/utils/panics/panics.go index 5a97d5a1bf..974264588f 100644 --- a/pkg/utils/panics/panics.go +++ b/pkg/utils/panics/panics.go @@ -13,9 +13,7 @@ import ( type PanicHandler func(interface{}) bool -var ( - PanicHandlers = []PanicHandler{logHandler} -) +var PanicHandlers = []PanicHandler{logHandler} func HandlePanic(additionalHandlers ...PanicHandler) { reallyCrash := true From 494a0480a017fc9816b7e7676908cdb5515dee77 Mon Sep 17 00:00:00 2001 From: Josh Michielsen Date: Thu, 2 Mar 2023 14:08:31 +0000 Subject: [PATCH 04/10] remove bool return from handlers, use variable to control crash --- .../common/options/closureoption/option.go | 2 -- pkg/utils/panics/panics.go | 16 +++++++--------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/cmds/ocm/commands/common/options/closureoption/option.go b/cmds/ocm/commands/common/options/closureoption/option.go index 5c5f03ad42..b1a47b7c71 100644 --- a/cmds/ocm/commands/common/options/closureoption/option.go +++ b/cmds/ocm/commands/common/options/closureoption/option.go @@ -16,7 +16,6 @@ import ( "github.com/open-component-model/ocm/pkg/contexts/ocm/transfer/transferhandler" "github.com/open-component-model/ocm/pkg/contexts/ocm/transfer/transferhandler/standard" "github.com/open-component-model/ocm/pkg/utils" - "github.com/open-component-model/ocm/pkg/utils/panics" ) func From(o options.OptionSetProvider) *Option { @@ -35,7 +34,6 @@ type Option struct { } func New(elemname string, settings ...interface{}) *Option { - defer panics.HandlePanic() o := &Option{ElementName: elemname, AddReferencePath: options.Always()} for _, s := range settings { switch v := s.(type) { diff --git a/pkg/utils/panics/panics.go b/pkg/utils/panics/panics.go index 974264588f..a76e31bd36 100644 --- a/pkg/utils/panics/panics.go +++ b/pkg/utils/panics/panics.go @@ -11,23 +11,23 @@ import ( "github.com/open-component-model/ocm/pkg/logging" ) -type PanicHandler func(interface{}) bool +var ReallyCrash = true + +type PanicHandler func(interface{}) var PanicHandlers = []PanicHandler{logHandler} func HandlePanic(additionalHandlers ...PanicHandler) { - reallyCrash := true - if r := recover(); r != nil { for _, fn := range PanicHandlers { - reallyCrash = fn(r) + fn(r) } for _, fn := range additionalHandlers { - reallyCrash = fn(r) + fn(r) } - if reallyCrash { + if ReallyCrash { panic(r) } } @@ -37,7 +37,7 @@ func RegisterPanicHandler(handler PanicHandler) { PanicHandlers = append(PanicHandlers, handler) } -func logHandler(r interface{}) bool { +func logHandler(r interface{}) { // Same as stdlib http server code. Manually allocate stack trace buffer size // to prevent excessively large logs const size = 64 << 10 @@ -48,6 +48,4 @@ func logHandler(r interface{}) bool { } else { logging.Logger().Error(fmt.Sprintf("Observed a panic: %#v (%v)\n%s", r, r, stacktrace)) } - - return true } From 3e8092be908ca198665274a7310f5a424f2eb6e6 Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Mon, 27 Mar 2023 16:40:49 +0200 Subject: [PATCH 05/10] removed some handlers and refactored others to return an error instead --- cmds/ocm/pkg/data/dll.go | 3 -- cmds/ocm/pkg/processing/buffer.go | 4 +++ cmds/ocm/pkg/processing/pool.go | 3 -- cmds/ocm/pkg/utils/handling.go | 1 + pkg/cobrautils/flag/string_to_string.go | 3 -- pkg/cobrautils/links.go | 1 + pkg/common/accessobj/accessstate.go | 2 -- pkg/contexts/datacontext/util.go | 9 +++--- pkg/contexts/oci/attrs/cacheattr/attr.go | 4 +-- pkg/contexts/oci/cpi/flavor_artifact.go | 4 +-- pkg/contexts/oci/cpi/flavor_index.go | 6 ++-- pkg/contexts/oci/cpi/flavor_manifest.go | 5 ++- .../oci/cpi/support/flavor_artifact.go | 4 +-- pkg/contexts/oci/repositories/ocireg/blobs.go | 5 ++- .../oci/repositories/ocireg/namespace.go | 31 +++++++++++++++---- .../v3alpha1/jsonscheme/jsonscheme.go | 3 -- .../versions/v2/jsonscheme/jsonscheme.go | 3 -- pkg/contexts/ocm/internal/builder.go | 4 +-- pkg/contexts/ocm/internal/builtin.go | 2 -- pkg/contexts/ocm/internal/digesthandler.go | 2 -- pkg/contexts/ocm/plugin/ppi/utils.go | 2 -- pkg/contexts/ocm/utils.go | 4 +-- pkg/regex/regex.go | 3 -- pkg/runtime/scheme.go | 3 -- pkg/runtime/utils.go | 2 -- pkg/runtime/versionedtype.go | 3 -- pkg/signing/handlers/rsa/format.go | 2 ++ pkg/signing/normalization.go | 2 -- pkg/utils/utils.go | 4 +-- 29 files changed, 50 insertions(+), 74 deletions(-) diff --git a/cmds/ocm/pkg/data/dll.go b/cmds/ocm/pkg/data/dll.go index 770e9e7862..f1b61d6078 100644 --- a/cmds/ocm/pkg/data/dll.go +++ b/cmds/ocm/pkg/data/dll.go @@ -6,8 +6,6 @@ package data import ( "sync" - - "github.com/open-component-model/ocm/pkg/utils/panics" ) type DLLRoot struct { @@ -80,7 +78,6 @@ func (this *DLL) Set(p interface{}) { } func (this *DLL) Append(d *DLL) { - defer panics.HandlePanic() if d.next != nil || d.prev != nil { panic("dll element already in use") } diff --git a/cmds/ocm/pkg/processing/buffer.go b/cmds/ocm/pkg/processing/buffer.go index 88d91b1318..78a8ba81e9 100644 --- a/cmds/ocm/pkg/processing/buffer.go +++ b/cmds/ocm/pkg/processing/buffer.go @@ -73,6 +73,8 @@ type ProcessingEntry struct { type SubEntries int func NewEntry(i Index, v interface{}, opts ...interface{}) ProcessingEntry { + // If this is caught the only upstream problem would be an empty entry. + // Which is fine if the user understands that it can happen. defer panics.HandlePanic() max := -1 sub := 0 @@ -367,6 +369,8 @@ func (ob *orderedBuffer) SetFrame(frame BufferFrame) { } func (ob *orderedBuffer) Add(e ProcessingEntry) bool { + // This handler here prevents the calling function's lock to be not released. + // The resulting state would return false, which is acceptable from the user's perspective. defer panics.HandlePanic() e.Index.Validate(e.MaxIndex) ob.simple.Add(e) diff --git a/cmds/ocm/pkg/processing/pool.go b/cmds/ocm/pkg/processing/pool.go index 6a8bf89753..5db6742ce9 100644 --- a/cmds/ocm/pkg/processing/pool.go +++ b/cmds/ocm/pkg/processing/pool.go @@ -6,8 +6,6 @@ package processing import ( "sync" - - "github.com/open-component-model/ocm/pkg/utils/panics" ) type ProcessorPool interface { @@ -57,7 +55,6 @@ func (this *_ProcessorPool) Request() { } func (this *_ProcessorPool) Exec(f func()) { - defer panics.HandlePanic() this.lock.Lock() defer this.lock.Unlock() if this.uses == 0 { diff --git a/cmds/ocm/pkg/utils/handling.go b/cmds/ocm/pkg/utils/handling.go index 7802f7b1bb..320882dd68 100644 --- a/cmds/ocm/pkg/utils/handling.go +++ b/cmds/ocm/pkg/utils/handling.go @@ -46,6 +46,7 @@ func StringElemSpecs(args ...string) []ElemSpec { } func ElemSpecs(list interface{}) []ElemSpec { + // This is acceptable as the calling chain will just take a nil slice. defer panics.HandlePanic() if list == nil { return nil diff --git a/pkg/cobrautils/flag/string_to_string.go b/pkg/cobrautils/flag/string_to_string.go index acf8757bc6..c20948070b 100644 --- a/pkg/cobrautils/flag/string_to_string.go +++ b/pkg/cobrautils/flag/string_to_string.go @@ -14,8 +14,6 @@ import ( "strings" "github.com/spf13/pflag" - - "github.com/open-component-model/ocm/pkg/utils/panics" ) type stringToStringValue[T ~map[string]string] struct { @@ -75,7 +73,6 @@ func (s *stringToStringValue[T]) Type() string { } func (s *stringToStringValue[T]) String() string { - defer panics.HandlePanic() records := make([]string, 0, len(*s.value)>>1) for k, v := range *s.value { records = append(records, k+"="+v) diff --git a/pkg/cobrautils/links.go b/pkg/cobrautils/links.go index bc56c0d324..e63cdef37b 100644 --- a/pkg/cobrautils/links.go +++ b/pkg/cobrautils/links.go @@ -34,6 +34,7 @@ func FormatLinkWithHandler(linkhandler func(string) string) func(string) string } func SubstituteCommandLinks(desc string, linkformat func(string) string) ([]string, string) { + // This is acceptable as the call chain will just output empty. defer panics.HandlePanic() var links []string for { diff --git a/pkg/common/accessobj/accessstate.go b/pkg/common/accessobj/accessstate.go index 90ffef7a69..81b4ddbf04 100644 --- a/pkg/common/accessobj/accessstate.go +++ b/pkg/common/accessobj/accessstate.go @@ -14,7 +14,6 @@ import ( "github.com/open-component-model/ocm/pkg/common/accessio" "github.com/open-component-model/ocm/pkg/errors" - "github.com/open-component-model/ocm/pkg/utils/panics" ) // These objects deal with descriptor based state descriptions @@ -222,7 +221,6 @@ func (s *state) Refresh() error { } func (s *state) GetOriginalState() interface{} { - defer panics.HandlePanic() if s.originalBlob == nil { return nil } diff --git a/pkg/contexts/datacontext/util.go b/pkg/contexts/datacontext/util.go index c9ef282c39..749f6f00af 100644 --- a/pkg/contexts/datacontext/util.go +++ b/pkg/contexts/datacontext/util.go @@ -7,8 +7,6 @@ package datacontext import ( "context" "fmt" - - "github.com/open-component-model/ocm/pkg/utils/panics" ) // ForContextByKey retrieves the context for a given key to be used for a context.Context. @@ -28,8 +26,7 @@ type ElementCopyable[T any] interface { type ElementCreator[T any] func(base ...T) T -func SetupElement[T ElementCopyable[T]](mode BuilderMode, target *T, create ElementCreator[T], def T) { - defer panics.HandlePanic() +func SetupElement[T ElementCopyable[T]](mode BuilderMode, target *T, create ElementCreator[T], def T) error { var zero T if *target == zero { switch mode { @@ -44,7 +41,9 @@ func SetupElement[T ElementCopyable[T]](mode BuilderMode, target *T, create Elem case MODE_SHARED: *target = def default: - panic(fmt.Sprintf("invalid context creation mode %s", mode)) + return fmt.Errorf("invalid context creation mode %s", mode) } } + + return nil } diff --git a/pkg/contexts/oci/attrs/cacheattr/attr.go b/pkg/contexts/oci/attrs/cacheattr/attr.go index b6917ccc9f..6e4dd296ac 100644 --- a/pkg/contexts/oci/attrs/cacheattr/attr.go +++ b/pkg/contexts/oci/attrs/cacheattr/attr.go @@ -13,7 +13,6 @@ import ( "github.com/open-component-model/ocm/pkg/contexts/datacontext" "github.com/open-component-model/ocm/pkg/errors" "github.com/open-component-model/ocm/pkg/runtime" - "github.com/open-component-model/ocm/pkg/utils/panics" ) const ( @@ -46,14 +45,13 @@ func (a AttributeType) Encode(v interface{}, marshaller runtime.Marshaler) ([]by } func (a AttributeType) Decode(data []byte, unmarshaller runtime.Unmarshaler) (interface{}, error) { - defer panics.HandlePanic() var value string err := unmarshaller.Unmarshal(data, &value) if value != "" { if strings.HasPrefix(value, "~"+string(os.PathSeparator)) { home := os.Getenv("HOME") if home == "" { - panic("HOME not set") + return nil, fmt.Errorf("HOME not set") } value = home + value[1:] } diff --git a/pkg/contexts/oci/cpi/flavor_artifact.go b/pkg/contexts/oci/cpi/flavor_artifact.go index 92f44fa487..7d9f669489 100644 --- a/pkg/contexts/oci/cpi/flavor_artifact.go +++ b/pkg/contexts/oci/cpi/flavor_artifact.go @@ -14,7 +14,6 @@ import ( "github.com/open-component-model/ocm/pkg/contexts/oci/artdesc" "github.com/open-component-model/ocm/pkg/contexts/oci/internal" "github.com/open-component-model/ocm/pkg/errors" - "github.com/open-component-model/ocm/pkg/utils/panics" ) var ErrNoIndex = errors.New("manifest does not support access to subsequent artifacts") @@ -68,7 +67,6 @@ func NewArtifactForBlob(access ArtifactSetContainer, blob accessio.BlobAccess) ( } func NewArtifact(access ArtifactSetContainer, defs ...*artdesc.Artifact) (ArtifactAccess, error) { - defer panics.HandlePanic() var def *artdesc.Artifact if len(defs) != 0 && defs[0] != nil { def = defs[0] @@ -79,7 +77,7 @@ func NewArtifact(access ArtifactSetContainer, defs ...*artdesc.Artifact) (Artifa } state, err := accessobj.NewBlobStateForObject(mode, def, NewArtifactStateHandler()) if err != nil { - panic("oops: " + err.Error()) + return nil, err } p, err := access.NewArtifactProvider(state) diff --git a/pkg/contexts/oci/cpi/flavor_index.go b/pkg/contexts/oci/cpi/flavor_index.go index 84bf4303b9..9d4f2e56de 100644 --- a/pkg/contexts/oci/cpi/flavor_index.go +++ b/pkg/contexts/oci/cpi/flavor_index.go @@ -5,6 +5,8 @@ package cpi import ( + "fmt" + "github.com/opencontainers/go-digest" "github.com/open-component-model/ocm/pkg/common/accessio" @@ -12,7 +14,6 @@ import ( "github.com/open-component-model/ocm/pkg/contexts/oci/artdesc" "github.com/open-component-model/ocm/pkg/contexts/oci/internal" "github.com/open-component-model/ocm/pkg/errors" - "github.com/open-component-model/ocm/pkg/utils/panics" ) type IndexImpl struct { @@ -22,7 +23,6 @@ type IndexImpl struct { var _ IndexAccess = (*IndexImpl)(nil) func NewIndex(access ArtifactSetContainer, defs ...*artdesc.Index) (internal.IndexAccess, error) { - defer panics.HandlePanic() var def *artdesc.Index if len(defs) != 0 && defs[0] != nil { def = defs[0] @@ -33,7 +33,7 @@ func NewIndex(access ArtifactSetContainer, defs ...*artdesc.Index) (internal.Ind } state, err := accessobj.NewBlobStateForObject(mode, def, NewIndexStateHandler()) if err != nil { - panic("oops") + return nil, fmt.Errorf("failed to get blob state for object: %w", err) } p, err := access.NewArtifactProvider(state) diff --git a/pkg/contexts/oci/cpi/flavor_manifest.go b/pkg/contexts/oci/cpi/flavor_manifest.go index ed63902e91..59e39998f0 100644 --- a/pkg/contexts/oci/cpi/flavor_manifest.go +++ b/pkg/contexts/oci/cpi/flavor_manifest.go @@ -6,6 +6,7 @@ package cpi import ( "compress/gzip" + "fmt" "github.com/opencontainers/go-digest" @@ -13,7 +14,6 @@ import ( "github.com/open-component-model/ocm/pkg/common/accessobj" "github.com/open-component-model/ocm/pkg/contexts/oci/artdesc" "github.com/open-component-model/ocm/pkg/errors" - "github.com/open-component-model/ocm/pkg/utils/panics" ) type ManifestImpl struct { @@ -23,7 +23,6 @@ type ManifestImpl struct { var _ ManifestAccess = (*ManifestImpl)(nil) func NewManifest(access ArtifactSetContainer, defs ...*artdesc.Manifest) (*ManifestImpl, error) { - defer panics.HandlePanic() var def *artdesc.Manifest if len(defs) != 0 && defs[0] != nil { def = defs[0] @@ -34,7 +33,7 @@ func NewManifest(access ArtifactSetContainer, defs ...*artdesc.Manifest) (*Manif } state, err := accessobj.NewBlobStateForObject(mode, def, NewManifestStateHandler()) if err != nil { - panic("oops") + return nil, fmt.Errorf("failed to get blob state for object: %w", err) } p, err := access.NewArtifactProvider(state) diff --git a/pkg/contexts/oci/cpi/support/flavor_artifact.go b/pkg/contexts/oci/cpi/support/flavor_artifact.go index 8410eae83f..2e235678fc 100644 --- a/pkg/contexts/oci/cpi/support/flavor_artifact.go +++ b/pkg/contexts/oci/cpi/support/flavor_artifact.go @@ -15,7 +15,6 @@ import ( "github.com/open-component-model/ocm/pkg/contexts/oci/cpi" "github.com/open-component-model/ocm/pkg/contexts/oci/internal" "github.com/open-component-model/ocm/pkg/errors" - "github.com/open-component-model/ocm/pkg/utils/panics" ) var ErrNoIndex = errors.New("manifest does not support access to subsequent artifacts") @@ -40,7 +39,6 @@ func NewArtifactForBlob(container ArtifactSetContainerImpl, blob accessio.BlobAc } func NewArtifact(container ArtifactSetContainerImpl, defs ...*artdesc.Artifact) (cpi.ArtifactAccess, error) { - defer panics.HandlePanic() var def *artdesc.Artifact if len(defs) != 0 && defs[0] != nil { def = defs[0] @@ -51,7 +49,7 @@ func NewArtifact(container ArtifactSetContainerImpl, defs ...*artdesc.Artifact) } state, err := accessobj.NewBlobStateForObject(mode, def, cpi.NewArtifactStateHandler()) if err != nil { - panic("oops: " + err.Error()) + return nil, fmt.Errorf("failed to fetch new blob state: %w", err) } return newArtifactImpl(container, state) } diff --git a/pkg/contexts/oci/repositories/ocireg/blobs.go b/pkg/contexts/oci/repositories/ocireg/blobs.go index 4368211398..28aaae7465 100644 --- a/pkg/contexts/oci/repositories/ocireg/blobs.go +++ b/pkg/contexts/oci/repositories/ocireg/blobs.go @@ -16,7 +16,6 @@ import ( "github.com/open-component-model/ocm/pkg/contexts/oci/cpi" "github.com/open-component-model/ocm/pkg/docker/resolve" "github.com/open-component-model/ocm/pkg/errors" - "github.com/open-component-model/ocm/pkg/utils/panics" ) type BlobContainer interface { @@ -80,7 +79,6 @@ func newBlobContainer(mime string, fetcher resolve.Fetcher, pusher resolve.Pushe } func NewBlobContainer(cache accessio.BlobCache, mime string, fetcher resolve.Fetcher, pusher resolve.Pusher) BlobContainer { - defer panics.HandlePanic() c := newBlobContainer(mime, fetcher, pusher) if cache == nil { @@ -88,7 +86,8 @@ func NewBlobContainer(cache accessio.BlobCache, mime string, fetcher resolve.Fet } r, err := accessio.CachedAccess(c, c, cache) if err != nil { - panic(err) + // The calling code chain handles this blob container being nil. + return nil } return r } diff --git a/pkg/contexts/oci/repositories/ocireg/namespace.go b/pkg/contexts/oci/repositories/ocireg/namespace.go index a9877d3b9e..45fb84460a 100644 --- a/pkg/contexts/oci/repositories/ocireg/namespace.go +++ b/pkg/contexts/oci/repositories/ocireg/namespace.go @@ -120,7 +120,11 @@ func (n *NamespaceContainer) GetBlobDescriptor(digest digest.Digest) *cpi.Descri func (n *NamespaceContainer) GetBlobData(digest digest.Digest) (int64, cpi.DataAccess, error) { n.repo.ctx.Logger().Debug("getting blob", "digest", digest) - size, acc, err := n.blobs.Get("").GetBlobData(digest) + blob := n.blobs.Get("") + if blob == nil { + return -1, nil, fmt.Errorf("failed to retrieve blob data, blob data was nil") + } + size, acc, err := blob.GetBlobData(digest) n.repo.ctx.Logger().Debug("getting blob done", "digest", digest, "size", size, "error", logging.ErrorMessage(err)) return size, acc, err } @@ -128,7 +132,11 @@ func (n *NamespaceContainer) GetBlobData(digest digest.Digest) (int64, cpi.DataA func (n *NamespaceContainer) AddBlob(blob cpi.BlobAccess) error { log := n.repo.ctx.Logger() log.Debug("adding blob", "digest", blob.Digest()) - if _, _, err := n.blobs.Get("").AddBlob(blob); err != nil { + blobData := n.blobs.Get("") + if blobData == nil { + return fmt.Errorf("failed to retrieve blob data, blob data was empty") + } + if _, _, err := blobData.AddBlob(blob); err != nil { log.Debug("adding blob failed", "digest", blob.Digest(), "error", err.Error()) return fmt.Errorf("unable to add blob: %w", err) } @@ -151,7 +159,11 @@ func (n *NamespaceContainer) GetArtifact(vers string) (cpi.ArtifactAccess, error } return nil, err } - _, acc, err := n.blobs.Get(desc.MediaType).GetBlobData(desc.Digest) + blobData := n.blobs.Get(desc.MediaType) + if blobData == nil { + return nil, fmt.Errorf("failed to retrieve blob data, blob data was empty") + } + _, acc, err := blobData.GetBlobData(desc.Digest) if err != nil { return nil, err } @@ -163,18 +175,25 @@ func (n *NamespaceContainer) AddArtifact(artifact cpi.Artifact, tags ...string) if err != nil { return nil, err } + if n.repo.info.Legacy { blob = artdesc.MapArtifactBlobMimeType(blob, true) } + n.repo.ctx.Logger().Debug("adding artifact", "digest", blob.Digest(), "mimetype", blob.MimeType()) - _, _, err = n.blobs.Get(blob.MimeType()).AddBlob(blob) + blobData := n.blobs.Get(blob.MimeType()) + if blobData == nil { + return nil, fmt.Errorf("failed to retrieve blob data, blob data was empty") + } + + _, _, err = blobData.AddBlob(blob) if err != nil { return nil, err } + if len(tags) > 0 { for _, tag := range tags { - err := n.push(tag, blob) - if err != nil { + if err := n.push(tag, blob); err != nil { return nil, err } } diff --git a/pkg/contexts/ocm/compdesc/versions/ocm.software/v3alpha1/jsonscheme/jsonscheme.go b/pkg/contexts/ocm/compdesc/versions/ocm.software/v3alpha1/jsonscheme/jsonscheme.go index b19aba6d97..db3bdbfcaa 100644 --- a/pkg/contexts/ocm/compdesc/versions/ocm.software/v3alpha1/jsonscheme/jsonscheme.go +++ b/pkg/contexts/ocm/compdesc/versions/ocm.software/v3alpha1/jsonscheme/jsonscheme.go @@ -13,14 +13,11 @@ import ( "github.com/ghodss/yaml" "github.com/xeipuuv/gojsonschema" - - "github.com/open-component-model/ocm/pkg/utils/panics" ) var Schema *gojsonschema.Schema func init() { - defer panics.HandlePanic() dataBytes, err := ResourcesComponentDescriptorOcmV3SchemaYamlBytes() if err != nil { panic(err) diff --git a/pkg/contexts/ocm/compdesc/versions/v2/jsonscheme/jsonscheme.go b/pkg/contexts/ocm/compdesc/versions/v2/jsonscheme/jsonscheme.go index b662cdddb9..95e3fdc970 100644 --- a/pkg/contexts/ocm/compdesc/versions/v2/jsonscheme/jsonscheme.go +++ b/pkg/contexts/ocm/compdesc/versions/v2/jsonscheme/jsonscheme.go @@ -13,14 +13,11 @@ import ( "github.com/ghodss/yaml" "github.com/xeipuuv/gojsonschema" - - "github.com/open-component-model/ocm/pkg/utils/panics" ) var Schema *gojsonschema.Schema func init() { - defer panics.HandlePanic() dataBytes, err := ResourcesComponentDescriptorV2SchemaYamlBytes() if err != nil { panic(err) diff --git a/pkg/contexts/ocm/internal/builder.go b/pkg/contexts/ocm/internal/builder.go index af132245e6..162c4fe4c3 100644 --- a/pkg/contexts/ocm/internal/builder.go +++ b/pkg/contexts/ocm/internal/builder.go @@ -13,7 +13,6 @@ import ( "github.com/open-component-model/ocm/pkg/contexts/datacontext" "github.com/open-component-model/ocm/pkg/contexts/oci" "github.com/open-component-model/ocm/pkg/runtime" - "github.com/open-component-model/ocm/pkg/utils/panics" ) type Builder struct { @@ -184,14 +183,13 @@ type delegatingDecoder struct { var _ runtime.TypedObjectDecoder = (*delegatingDecoder)(nil) func (d *delegatingDecoder) Decode(data []byte, unmarshaler runtime.Unmarshaler) (runtime.TypedObject, error) { - defer panics.HandlePanic() d.lock.Lock() defer d.lock.Unlock() if d.delegate == nil && ociimpl != nil { def, err := ociimpl(d.oci) if err != nil { - panic(fmt.Sprintf("cannot create oci default decoder: %s", err)) + return nil, fmt.Errorf("cannot create oci default decoder: %w", err) } d.delegate = def } diff --git a/pkg/contexts/ocm/internal/builtin.go b/pkg/contexts/ocm/internal/builtin.go index 69618bf16f..fb0ef94e65 100644 --- a/pkg/contexts/ocm/internal/builtin.go +++ b/pkg/contexts/ocm/internal/builtin.go @@ -6,7 +6,6 @@ package internal import ( "github.com/open-component-model/ocm/pkg/contexts/oci" - "github.com/open-component-model/ocm/pkg/utils/panics" ) // @@ -17,7 +16,6 @@ type OCISpecFunction func(ctx oci.Context) (RepositoryType, error) var ociimpl OCISpecFunction func RegisterOCIImplementation(impl OCISpecFunction) { - defer panics.HandlePanic() if ociimpl != nil { panic("oci implementation already registered") } diff --git a/pkg/contexts/ocm/internal/digesthandler.go b/pkg/contexts/ocm/internal/digesthandler.go index 6d563189ba..b41f2c6d81 100644 --- a/pkg/contexts/ocm/internal/digesthandler.go +++ b/pkg/contexts/ocm/internal/digesthandler.go @@ -12,7 +12,6 @@ import ( metav1 "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/meta/v1" "github.com/open-component-model/ocm/pkg/signing" "github.com/open-component-model/ocm/pkg/utils" - "github.com/open-component-model/ocm/pkg/utils/panics" ) type DigesterType struct { @@ -260,7 +259,6 @@ func (r *blobDigesterRegistry) DetermineDigests(restype string, preferred signin } func MustRegisterDigester(digester BlobDigester, arttypes ...string) { - defer panics.HandlePanic() err := DefaultBlobDigesterRegistry.Register(digester, arttypes...) if err != nil { panic(err) diff --git a/pkg/contexts/ocm/plugin/ppi/utils.go b/pkg/contexts/ocm/plugin/ppi/utils.go index 671a9ad167..e41407a67e 100644 --- a/pkg/contexts/ocm/plugin/ppi/utils.go +++ b/pkg/contexts/ocm/plugin/ppi/utils.go @@ -6,7 +6,6 @@ package ppi import ( "github.com/open-component-model/ocm/pkg/runtime" - "github.com/open-component-model/ocm/pkg/utils/panics" ) type decoder runtime.TypedObjectDecoder @@ -20,7 +19,6 @@ type AccessMethodBase struct { } func MustNewAccessMethodBase(name, version string, proto AccessSpec, desc string, format string) AccessMethodBase { - defer panics.HandlePanic() decoder, err := runtime.NewDirectDecoder(proto) if err != nil { panic(err) diff --git a/pkg/contexts/ocm/utils.go b/pkg/contexts/ocm/utils.go index 37735943b4..2a2e7d3123 100644 --- a/pkg/contexts/ocm/utils.go +++ b/pkg/contexts/ocm/utils.go @@ -15,13 +15,11 @@ import ( "github.com/open-component-model/ocm/pkg/contexts/ocm/cpi" "github.com/open-component-model/ocm/pkg/contexts/ocm/repositories/ctf" "github.com/open-component-model/ocm/pkg/errors" - "github.com/open-component-model/ocm/pkg/utils/panics" ) //////////////////////////////////////////////////////////////////////////////// func AssureTargetRepository(session Session, ctx Context, targetref string, opts ...interface{}) (Repository, error) { - defer panics.HandlePanic() var format accessio.FileFormat var archive string var fs vfs.FileSystem @@ -37,7 +35,7 @@ func AssureTargetRepository(session Session, ctx Context, targetref string, opts case string: archive = v default: - panic(fmt.Sprintf("invalid option type %T", o)) + return nil, fmt.Errorf("invalid option type %T", o) } } diff --git a/pkg/regex/regex.go b/pkg/regex/regex.go index 1aab10fbc6..1e4bf0b9e0 100644 --- a/pkg/regex/regex.go +++ b/pkg/regex/regex.go @@ -6,8 +6,6 @@ package regex import ( "regexp" - - "github.com/open-component-model/ocm/pkg/utils/panics" ) var ( @@ -29,7 +27,6 @@ var Match = regexp.MustCompile // Literal compiles s into a literal regular expression, escaping any regexp // reserved characters. func Literal(s string) *regexp.Regexp { - defer panics.HandlePanic() re := Match(regexp.QuoteMeta(s)) if _, complete := re.LiteralPrefix(); !complete { diff --git a/pkg/runtime/scheme.go b/pkg/runtime/scheme.go index b04d7abb46..80f89353de 100644 --- a/pkg/runtime/scheme.go +++ b/pkg/runtime/scheme.go @@ -13,7 +13,6 @@ import ( "github.com/open-component-model/ocm/pkg/errors" "github.com/open-component-model/ocm/pkg/utils" - "github.com/open-component-model/ocm/pkg/utils/panics" ) // TypeGetter is the interface to be implemented for extracting a type. @@ -54,7 +53,6 @@ type DirectDecoder struct { var _ TypedObjectDecoder = &DirectDecoder{} func MustNewDirectDecoder(proto interface{}) *DirectDecoder { - defer panics.HandlePanic() d, err := NewDirectDecoder(proto) if err != nil { panic(err) @@ -109,7 +107,6 @@ type ConvertingDecoder struct { var _ TypedObjectDecoder = &ConvertingDecoder{} func MustNewConvertingDecoder(proto interface{}, conv TypedObjectConverter) *ConvertingDecoder { - defer panics.HandlePanic() d, err := NewConvertingDecoder(proto, conv) if err != nil { panic(err) diff --git a/pkg/runtime/utils.go b/pkg/runtime/utils.go index e05e841bc9..f8c1d0d27b 100644 --- a/pkg/runtime/utils.go +++ b/pkg/runtime/utils.go @@ -10,11 +10,9 @@ import ( "strings" "github.com/open-component-model/ocm/pkg/errors" - "github.com/open-component-model/ocm/pkg/utils/panics" ) func MustProtoType(proto interface{}) reflect.Type { - defer panics.HandlePanic() t, err := ProtoType(proto) if err != nil { panic(err.Error()) diff --git a/pkg/runtime/versionedtype.go b/pkg/runtime/versionedtype.go index bd0c77d666..ac3aa01876 100644 --- a/pkg/runtime/versionedtype.go +++ b/pkg/runtime/versionedtype.go @@ -6,8 +6,6 @@ package runtime import ( "strings" - - "github.com/open-component-model/ocm/pkg/utils/panics" ) const VersionSeparator = "/" @@ -19,7 +17,6 @@ type VersionedTypedObject interface { } func TypeName(args ...string) string { - defer panics.HandlePanic() if len(args) == 1 { return args[0] } diff --git a/pkg/signing/handlers/rsa/format.go b/pkg/signing/handlers/rsa/format.go index e3b77ead47..01e8bb3d0f 100644 --- a/pkg/signing/handlers/rsa/format.go +++ b/pkg/signing/handlers/rsa/format.go @@ -68,6 +68,8 @@ func KeyData(key interface{}) ([]byte, error) { } func PemBlockForKey(priv interface{}, gen ...bool) *pem.Block { + // Handling the panic here means that the above write will error and return that error which might be + // acceptable by the end user. defer panics.HandlePanic() switch k := priv.(type) { case *rsa.PublicKey: diff --git a/pkg/signing/normalization.go b/pkg/signing/normalization.go index 32fa107065..59509c1a8f 100644 --- a/pkg/signing/normalization.go +++ b/pkg/signing/normalization.go @@ -13,7 +13,6 @@ import ( "github.com/open-component-model/ocm/pkg/errors" "github.com/open-component-model/ocm/pkg/utils" - "github.com/open-component-model/ocm/pkg/utils/panics" ) type Entries []Entry @@ -43,7 +42,6 @@ func (l Entries) ToString(gap string) string { } func toString(v interface{}, gap string) string { - defer panics.HandlePanic() switch castIn := v.(type) { case Entries: return castIn.ToString(gap) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index e8ad34f5a8..9858a9d3d3 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -28,7 +28,6 @@ import ( "sigs.k8s.io/yaml" ocmlog "github.com/open-component-model/ocm/pkg/logging" - "github.com/open-component-model/ocm/pkg/utils/panics" ) // PrintPrettyYaml prints the given objects as yaml if enabled. @@ -295,9 +294,8 @@ func GetOptionFlag(list ...bool) bool { return OptionalDefaultedBool(len(list) == 0, list...) } -// Must expects a result to be provided without error. +// Must expect a result to be provided without error. func Must[T any](o T, err error) T { - defer panics.HandlePanic() if err != nil { panic(fmt.Errorf("expected a %T, but got error %w", o, err)) } From b2606b4d8f77c7207bbfc01363fe71f8ff97a475 Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Tue, 28 Mar 2023 12:39:24 +0200 Subject: [PATCH 06/10] changed Get to return an error instead removing the panic completely --- cmds/ocm/pkg/processing/buffer.go | 7 +++--- cmds/ocm/pkg/utils/handling.go | 1 + pkg/cobrautils/links.go | 4 ---- pkg/contexts/oci/repositories/ocireg/blobs.go | 23 +++++++++++------- .../oci/repositories/ocireg/namespace.go | 24 +++++++++---------- pkg/utils/panics/panics.go | 20 ++++++++++++---- 6 files changed, 45 insertions(+), 34 deletions(-) diff --git a/cmds/ocm/pkg/processing/buffer.go b/cmds/ocm/pkg/processing/buffer.go index 78a8ba81e9..b10a7cbb5a 100644 --- a/cmds/ocm/pkg/processing/buffer.go +++ b/cmds/ocm/pkg/processing/buffer.go @@ -75,7 +75,9 @@ type SubEntries int func NewEntry(i Index, v interface{}, opts ...interface{}) ProcessingEntry { // If this is caught the only upstream problem would be an empty entry. // Which is fine if the user understands that it can happen. - defer panics.HandlePanic() + defer func() { + _ = panics.HandlePanic() + }() max := -1 sub := 0 valid := true @@ -369,9 +371,6 @@ func (ob *orderedBuffer) SetFrame(frame BufferFrame) { } func (ob *orderedBuffer) Add(e ProcessingEntry) bool { - // This handler here prevents the calling function's lock to be not released. - // The resulting state would return false, which is acceptable from the user's perspective. - defer panics.HandlePanic() e.Index.Validate(e.MaxIndex) ob.simple.Add(e) n := data.NewDLL(&e) diff --git a/cmds/ocm/pkg/utils/handling.go b/cmds/ocm/pkg/utils/handling.go index 320882dd68..3f4d0479cb 100644 --- a/cmds/ocm/pkg/utils/handling.go +++ b/cmds/ocm/pkg/utils/handling.go @@ -48,6 +48,7 @@ func StringElemSpecs(args ...string) []ElemSpec { func ElemSpecs(list interface{}) []ElemSpec { // This is acceptable as the calling chain will just take a nil slice. defer panics.HandlePanic() + if list == nil { return nil } diff --git a/pkg/cobrautils/links.go b/pkg/cobrautils/links.go index e63cdef37b..2257e9dc6f 100644 --- a/pkg/cobrautils/links.go +++ b/pkg/cobrautils/links.go @@ -9,8 +9,6 @@ import ( "strings" "github.com/spf13/cobra" - - "github.com/open-component-model/ocm/pkg/utils/panics" ) func LinkForCmd(cmd *cobra.Command) string { @@ -34,8 +32,6 @@ func FormatLinkWithHandler(linkhandler func(string) string) func(string) string } func SubstituteCommandLinks(desc string, linkformat func(string) string) ([]string, string) { - // This is acceptable as the call chain will just output empty. - defer panics.HandlePanic() var links []string for { link := strings.Index(desc, "") diff --git a/pkg/contexts/oci/repositories/ocireg/blobs.go b/pkg/contexts/oci/repositories/ocireg/blobs.go index 28aaae7465..185532d110 100644 --- a/pkg/contexts/oci/repositories/ocireg/blobs.go +++ b/pkg/contexts/oci/repositories/ocireg/blobs.go @@ -48,16 +48,22 @@ func NewBlobContainers(ctx cpi.Context, fetcher remotes.Fetcher, pusher resolve. } } -func (c *BlobContainers) Get(mime string) BlobContainer { +func (c *BlobContainers) Get(mime string) (BlobContainer, error) { c.lock.Lock() defer c.lock.Unlock() found := c.mimes[mime] if found == nil { - found = NewBlobContainer(c.cache, mime, c.fetcher, c.pusher) - c.mimes[mime] = found + container, err := NewBlobContainer(c.cache, mime, c.fetcher, c.pusher) + if err != nil { + return nil, err + } + c.mimes[mime] = container + + return container, nil } - return found + + return found, nil } func (c *BlobContainers) Release() error { @@ -78,18 +84,17 @@ func newBlobContainer(mime string, fetcher resolve.Fetcher, pusher resolve.Pushe } } -func NewBlobContainer(cache accessio.BlobCache, mime string, fetcher resolve.Fetcher, pusher resolve.Pusher) BlobContainer { +func NewBlobContainer(cache accessio.BlobCache, mime string, fetcher resolve.Fetcher, pusher resolve.Pusher) (BlobContainer, error) { c := newBlobContainer(mime, fetcher, pusher) if cache == nil { - return c + return c, nil } r, err := accessio.CachedAccess(c, c, cache) if err != nil { - // The calling code chain handles this blob container being nil. - return nil + return nil, err } - return r + return r, nil } func (n *blobContainer) GetBlobData(digest digest.Digest) (int64, cpi.DataAccess, error) { diff --git a/pkg/contexts/oci/repositories/ocireg/namespace.go b/pkg/contexts/oci/repositories/ocireg/namespace.go index 45fb84460a..b136e9fffe 100644 --- a/pkg/contexts/oci/repositories/ocireg/namespace.go +++ b/pkg/contexts/oci/repositories/ocireg/namespace.go @@ -120,9 +120,9 @@ func (n *NamespaceContainer) GetBlobDescriptor(digest digest.Digest) *cpi.Descri func (n *NamespaceContainer) GetBlobData(digest digest.Digest) (int64, cpi.DataAccess, error) { n.repo.ctx.Logger().Debug("getting blob", "digest", digest) - blob := n.blobs.Get("") - if blob == nil { - return -1, nil, fmt.Errorf("failed to retrieve blob data, blob data was nil") + blob, err := n.blobs.Get("") + if err != nil { + return -1, nil, fmt.Errorf("failed to retrieve blob data: %w", err) } size, acc, err := blob.GetBlobData(digest) n.repo.ctx.Logger().Debug("getting blob done", "digest", digest, "size", size, "error", logging.ErrorMessage(err)) @@ -132,9 +132,9 @@ func (n *NamespaceContainer) GetBlobData(digest digest.Digest) (int64, cpi.DataA func (n *NamespaceContainer) AddBlob(blob cpi.BlobAccess) error { log := n.repo.ctx.Logger() log.Debug("adding blob", "digest", blob.Digest()) - blobData := n.blobs.Get("") - if blobData == nil { - return fmt.Errorf("failed to retrieve blob data, blob data was empty") + blobData, err := n.blobs.Get("") + if err != nil { + return fmt.Errorf("failed to retrieve blob data: %w", err) } if _, _, err := blobData.AddBlob(blob); err != nil { log.Debug("adding blob failed", "digest", blob.Digest(), "error", err.Error()) @@ -159,9 +159,9 @@ func (n *NamespaceContainer) GetArtifact(vers string) (cpi.ArtifactAccess, error } return nil, err } - blobData := n.blobs.Get(desc.MediaType) - if blobData == nil { - return nil, fmt.Errorf("failed to retrieve blob data, blob data was empty") + blobData, err := n.blobs.Get(desc.MediaType) + if err != nil { + return nil, fmt.Errorf("failed to retrieve blob data, blob data was empty: %w", err) } _, acc, err := blobData.GetBlobData(desc.Digest) if err != nil { @@ -181,9 +181,9 @@ func (n *NamespaceContainer) AddArtifact(artifact cpi.Artifact, tags ...string) } n.repo.ctx.Logger().Debug("adding artifact", "digest", blob.Digest(), "mimetype", blob.MimeType()) - blobData := n.blobs.Get(blob.MimeType()) - if blobData == nil { - return nil, fmt.Errorf("failed to retrieve blob data, blob data was empty") + blobData, err := n.blobs.Get(blob.MimeType()) + if err != nil { + return nil, fmt.Errorf("failed to retrieve blob data: %w", err) } _, _, err = blobData.AddBlob(blob) diff --git a/pkg/utils/panics/panics.go b/pkg/utils/panics/panics.go index a76e31bd36..56ed54f7c1 100644 --- a/pkg/utils/panics/panics.go +++ b/pkg/utils/panics/panics.go @@ -9,35 +9,43 @@ import ( "runtime" "github.com/open-component-model/ocm/pkg/logging" + "github.com/pkg/errors" ) var ReallyCrash = true -type PanicHandler func(interface{}) +type PanicHandler func(interface{}) error var PanicHandlers = []PanicHandler{logHandler} -func HandlePanic(additionalHandlers ...PanicHandler) { +func HandlePanic(additionalHandlers ...PanicHandler) (err error) { if r := recover(); r != nil { for _, fn := range PanicHandlers { - fn(r) + if ferr := fn(r); ferr != nil { + err = errors.Wrap(err, ferr.Error()) + } } for _, fn := range additionalHandlers { - fn(r) + if ferr := fn(r); ferr != nil { + err = errors.Wrap(err, ferr.Error()) + } } if ReallyCrash { panic(r) } } + + return } +// RegisterPanicHandler adds handlers to the panic handler. func RegisterPanicHandler(handler PanicHandler) { PanicHandlers = append(PanicHandlers, handler) } -func logHandler(r interface{}) { +func logHandler(r interface{}) error { // Same as stdlib http server code. Manually allocate stack trace buffer size // to prevent excessively large logs const size = 64 << 10 @@ -48,4 +56,6 @@ func logHandler(r interface{}) { } else { logging.Logger().Error(fmt.Sprintf("Observed a panic: %#v (%v)\n%s", r, r, stacktrace)) } + + return nil } From bb3253fab2fac4c8dfeebb8a915ce01f9e071b14 Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Tue, 28 Mar 2023 12:57:57 +0200 Subject: [PATCH 07/10] return error instead of panicing --- cmds/ocm/commands/ocmcmds/references/get/cmd.go | 7 ++++++- cmds/ocm/commands/ocmcmds/resources/download/cmd.go | 7 ++++++- cmds/ocm/commands/ocmcmds/resources/get/cmd.go | 7 ++++++- cmds/ocm/commands/ocmcmds/sources/get/cmd.go | 7 ++++++- cmds/ocm/pkg/utils/handling.go | 12 ++++-------- pkg/utils/panics/panics.go | 3 ++- 6 files changed, 30 insertions(+), 13 deletions(-) diff --git a/cmds/ocm/commands/ocmcmds/references/get/cmd.go b/cmds/ocm/commands/ocmcmds/references/get/cmd.go index 7e75a1a34a..6a57bb89ef 100644 --- a/cmds/ocm/commands/ocmcmds/references/get/cmd.go +++ b/cmds/ocm/commands/ocmcmds/references/get/cmd.go @@ -78,7 +78,12 @@ func (o *Command) Run() error { if err != nil { return err } - return utils.HandleOutputs(opts, hdlr, utils.ElemSpecs(o.Ids)...) + specs, err := utils.ElemSpecs(o.Ids) + if err != nil { + return err + } + + return utils.HandleOutputs(opts, hdlr, specs...) } //////////////////////////////////////////////////////////////////////////////// diff --git a/cmds/ocm/commands/ocmcmds/resources/download/cmd.go b/cmds/ocm/commands/ocmcmds/resources/download/cmd.go index fe9c1b15d1..4f01326ccf 100644 --- a/cmds/ocm/commands/ocmcmds/resources/download/cmd.go +++ b/cmds/ocm/commands/ocmcmds/resources/download/cmd.go @@ -150,7 +150,12 @@ func (o *Command) Run() error { if err != nil { return err } - return utils.HandleOutputs(opts, hdlr, utils.ElemSpecs(o.Ids)...) + specs, err := utils.ElemSpecs(o.Ids) + if err != nil { + return err + } + + return utils.HandleOutputs(opts, hdlr, specs...) } //////////////////////////////////////////////////////////////////////////////// diff --git a/cmds/ocm/commands/ocmcmds/resources/get/cmd.go b/cmds/ocm/commands/ocmcmds/resources/get/cmd.go index 6ea50d3a1e..56851ad8da 100644 --- a/cmds/ocm/commands/ocmcmds/resources/get/cmd.go +++ b/cmds/ocm/commands/ocmcmds/resources/get/cmd.go @@ -76,7 +76,12 @@ func (o *Command) Run() error { if err != nil { return err } - return utils.HandleOutputs(opts, hdlr, utils.ElemSpecs(o.Ids)...) + specs, err := utils.ElemSpecs(o.Ids) + if err != nil { + return err + } + + return utils.HandleOutputs(opts, hdlr, specs...) } //////////////////////////////////////////////////////////////////////////////// diff --git a/cmds/ocm/commands/ocmcmds/sources/get/cmd.go b/cmds/ocm/commands/ocmcmds/sources/get/cmd.go index feb6579f1d..a9dd57d066 100644 --- a/cmds/ocm/commands/ocmcmds/sources/get/cmd.go +++ b/cmds/ocm/commands/ocmcmds/sources/get/cmd.go @@ -76,7 +76,12 @@ func (o *Command) Run() error { if err != nil { return err } - return utils.HandleOutputs(opts, hdlr, utils.ElemSpecs(o.Ids)...) + specs, err := utils.ElemSpecs(o.Ids) + if err != nil { + return err + } + + return utils.HandleOutputs(opts, hdlr, specs...) } //////////////////////////////////////////////////////////////////////////////// diff --git a/cmds/ocm/pkg/utils/handling.go b/cmds/ocm/pkg/utils/handling.go index 3f4d0479cb..44b4a76d8f 100644 --- a/cmds/ocm/pkg/utils/handling.go +++ b/cmds/ocm/pkg/utils/handling.go @@ -11,7 +11,6 @@ import ( "github.com/open-component-model/ocm/cmds/ocm/pkg/output" "github.com/open-component-model/ocm/pkg/errors" - "github.com/open-component-model/ocm/pkg/utils/panics" ) type ElemSpec interface { @@ -45,22 +44,19 @@ func StringElemSpecs(args ...string) []ElemSpec { return r } -func ElemSpecs(list interface{}) []ElemSpec { - // This is acceptable as the calling chain will just take a nil slice. - defer panics.HandlePanic() - +func ElemSpecs(list interface{}) ([]ElemSpec, error) { if list == nil { - return nil + return nil, nil } v := reflect.ValueOf(list) if v.Kind() != reflect.Array && v.Kind() != reflect.Slice { - panic("no array") + return nil, fmt.Errorf("kind was not an array but: %s", v.Kind().String()) } r := make([]ElemSpec, v.Len()) for i := 0; i < v.Len(); i++ { r[i] = v.Index(i).Interface().(ElemSpec) } - return r + return r, nil } func HandleArgs(opts *output.Options, handler TypeHandler, args ...string) error { diff --git a/pkg/utils/panics/panics.go b/pkg/utils/panics/panics.go index 56ed54f7c1..f6aa7145bc 100644 --- a/pkg/utils/panics/panics.go +++ b/pkg/utils/panics/panics.go @@ -8,8 +8,9 @@ import ( "fmt" "runtime" - "github.com/open-component-model/ocm/pkg/logging" "github.com/pkg/errors" + + "github.com/open-component-model/ocm/pkg/logging" ) var ReallyCrash = true From fd657ce73a696f657c464c884d1a13f6dfd3bb74 Mon Sep 17 00:00:00 2001 From: Uwe Krueger Date: Tue, 28 Mar 2023 17:02:21 +0200 Subject: [PATCH 08/10] support for mapping panic to error return --- cmds/ocm/pkg/processing/buffer.go | 5 +- pkg/utils/panics/panics.go | 76 +++++++++++++++++++++++-------- pkg/utils/panics/panics_test.go | 63 +++++++++++++++++++++++++ pkg/utils/panics/suite_test.go | 17 +++++++ 4 files changed, 138 insertions(+), 23 deletions(-) create mode 100644 pkg/utils/panics/panics_test.go create mode 100644 pkg/utils/panics/suite_test.go diff --git a/cmds/ocm/pkg/processing/buffer.go b/cmds/ocm/pkg/processing/buffer.go index b10a7cbb5a..3ae7e0fd91 100644 --- a/cmds/ocm/pkg/processing/buffer.go +++ b/cmds/ocm/pkg/processing/buffer.go @@ -75,9 +75,8 @@ type SubEntries int func NewEntry(i Index, v interface{}, opts ...interface{}) ProcessingEntry { // If this is caught the only upstream problem would be an empty entry. // Which is fine if the user understands that it can happen. - defer func() { - _ = panics.HandlePanic() - }() + defer panics.HandlePanic() + max := -1 sub := 0 valid := true diff --git a/pkg/utils/panics/panics.go b/pkg/utils/panics/panics.go index f6aa7145bc..da386cd081 100644 --- a/pkg/utils/panics/panics.go +++ b/pkg/utils/panics/panics.go @@ -7,9 +7,9 @@ package panics import ( "fmt" "runtime" + "strings" - "github.com/pkg/errors" - + "github.com/open-component-model/ocm/pkg/errors" "github.com/open-component-model/ocm/pkg/logging" ) @@ -19,26 +19,46 @@ type PanicHandler func(interface{}) error var PanicHandlers = []PanicHandler{logHandler} -func HandlePanic(additionalHandlers ...PanicHandler) (err error) { +// PropagatePanicAsError is intended to be called via defer to +// map a recovered panic to an error and propagate it to the +// error return value of the calling function. +// Use: defer panics.PropagatePanicAsError(&err, false) +// It incorporates the originally returned error by the surrounding function +// and all errors provided by the panic handlers plus the mapped recovered panic. +func PropagatePanicAsError(errp *error, callStandardHandlers bool, additionalHandlers ...PanicHandler) { if r := recover(); r != nil { - for _, fn := range PanicHandlers { - if ferr := fn(r); ferr != nil { - err = errors.Wrap(err, ferr.Error()) + list := errors.ErrList().Add(mapRecovered(r)) + + if callStandardHandlers { + // gather errors from standard handler + for _, fn := range PanicHandlers { + list.Add(fn(r)) } } + // add errors from explicit handlers for _, fn := range additionalHandlers { - if ferr := fn(r); ferr != nil { - err = errors.Wrap(err, ferr.Error()) - } + list.Add(fn(r)) + } + + *errp = list.Result() + } +} + +func HandlePanic(additionalHandlers ...PanicHandler) { + if r := recover(); r != nil { + for _, fn := range PanicHandlers { + _ = fn(r) + } + + for _, fn := range additionalHandlers { + _ = fn(r) } if ReallyCrash { panic(r) } } - - return } // RegisterPanicHandler adds handlers to the panic handler. @@ -46,17 +66,33 @@ func RegisterPanicHandler(handler PanicHandler) { PanicHandlers = append(PanicHandlers, handler) } -func logHandler(r interface{}) error { - // Same as stdlib http server code. Manually allocate stack trace buffer size - // to prevent excessively large logs - const size = 64 << 10 - stacktrace := make([]byte, size) - stacktrace = stacktrace[:runtime.Stack(stacktrace, false)] - if _, ok := r.(string); ok { - logging.Logger().Error(fmt.Sprintf("Observed a panic: %#v\n%s", r, stacktrace)) +func mapRecovered(r interface{}) error { + if err, ok := r.(error); ok { + return err } else { - logging.Logger().Error(fmt.Sprintf("Observed a panic: %#v (%v)\n%s", r, r, stacktrace)) + // Same as stdlib http server code. Manually allocate stack trace buffer size + // to prevent excessively large logs + const size = 64 << 10 + stacktrace := make([]byte, size) + stacktrace = stacktrace[:runtime.Stack(stacktrace, false)] + + stack := string(stacktrace) + lines := strings.Split(stack, "\n") + offset := 1 + for offset < len(lines) && !strings.HasPrefix(lines[offset], "panic(") { + offset++ + } + if offset < len(lines) { + stack = strings.Join(append(lines[:1], lines[offset:]...), "\n") + } + if _, ok := r.(string); ok { + return fmt.Errorf("Observed a panic: %#v\n%s", r, stack) + } + return fmt.Errorf("Observed a panic: %#v (%v)\n%s", r, r, stack) } +} +func logHandler(r interface{}) error { + logging.Logger().Error(mapRecovered(r).Error()) return nil } diff --git a/pkg/utils/panics/panics_test.go b/pkg/utils/panics/panics_test.go new file mode 100644 index 0000000000..2ff33400fc --- /dev/null +++ b/pkg/utils/panics/panics_test.go @@ -0,0 +1,63 @@ +// SPDX-FileCopyrightText: 2023 SAP SE or an SAP affiliate company and Open Component Model contributors. +// +// SPDX-License-Identifier: Apache-2.0 + +package panics_test + +import ( + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/open-component-model/ocm/pkg/utils/panics" +) + +func caller(topanic interface{}, outerr error, handlers ...panics.PanicHandler) (err error) { + defer panics.PropagatePanicAsError(&err, false, handlers...) + + err = outerr + callee(topanic) + return +} + +func callee(topanic interface{}) { + if topanic != nil { + panic(topanic) + } +} + +var _ = Describe("catch panics", func() { + It("propagates catched panic", func() { + defer func() { + Expect(recover()).To(BeNil()) + }() + err := caller("exception", nil) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(MatchRegexp(`(?s)Observed a panic: "exception" +goroutine [0-9]* \[running\]: +panic.*$`)) + }) + + FIt("propagates catched panic with handlers", func() { + defer func() { + Expect(recover()).To(BeNil()) + }() + err := caller("exception", nil, func(any) error { return fmt.Errorf("handler") }) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(MatchRegexp(`(?s){Observed a panic: "exception" +goroutine [0-9]* \[running\]: +panic.* +, handler}$`)) + }) + + It("regular error", func() { + defer func() { + Expect(recover()).To(BeNil()) + }() + err := caller(nil, fmt.Errorf("exception")) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("exception")) + }) + +}) diff --git a/pkg/utils/panics/suite_test.go b/pkg/utils/panics/suite_test.go new file mode 100644 index 0000000000..a7eb26cb8b --- /dev/null +++ b/pkg/utils/panics/suite_test.go @@ -0,0 +1,17 @@ +// SPDX-FileCopyrightText: 2022 SAP SE or an SAP affiliate company and Open Component Model contributors. +// +// SPDX-License-Identifier: Apache-2.0 + +package panics_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestConfig(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Panic Handling Test Suite") +} From 63c83234548c1f673fb2a0ca62a0280792699649 Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Wed, 29 Mar 2023 07:43:43 +0200 Subject: [PATCH 09/10] unfocused --- pkg/utils/panics/panics_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/utils/panics/panics_test.go b/pkg/utils/panics/panics_test.go index 2ff33400fc..6f14e21e96 100644 --- a/pkg/utils/panics/panics_test.go +++ b/pkg/utils/panics/panics_test.go @@ -39,7 +39,7 @@ goroutine [0-9]* \[running\]: panic.*$`)) }) - FIt("propagates catched panic with handlers", func() { + It("propagates catched panic with handlers", func() { defer func() { Expect(recover()).To(BeNil()) }() From a8eb475ffdab4fea86c217867ea93c32ae0893e1 Mon Sep 17 00:00:00 2001 From: Uwe Krueger Date: Wed, 29 Mar 2023 10:29:41 +0200 Subject: [PATCH 10/10] add error return for pem block --- pkg/signing/handlers/rsa/format.go | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/pkg/signing/handlers/rsa/format.go b/pkg/signing/handlers/rsa/format.go index 01e8bb3d0f..0219c71919 100644 --- a/pkg/signing/handlers/rsa/format.go +++ b/pkg/signing/handlers/rsa/format.go @@ -12,8 +12,8 @@ import ( "fmt" "io" + "github.com/open-component-model/ocm/pkg/errors" "github.com/open-component-model/ocm/pkg/utils" - "github.com/open-component-model/ocm/pkg/utils/panics" ) func GetPublicKey(key interface{}) (*rsa.PublicKey, []string, error) { @@ -56,21 +56,24 @@ func GetPrivateKey(key interface{}) (*rsa.PrivateKey, error) { } func WriteKeyData(key interface{}, w io.Writer) error { - block := PemBlockForKey(key) + block, err := PemBlockForKey(key) + if err != nil { + return err + } return pem.Encode(w, block) } func KeyData(key interface{}) ([]byte, error) { buf := bytes.NewBuffer(nil) - block := PemBlockForKey(key) - err := pem.Encode(buf, block) + block, err := PemBlockForKey(key) + if err != nil { + return nil, err + } + err = pem.Encode(buf, block) return buf.Bytes(), err } -func PemBlockForKey(priv interface{}, gen ...bool) *pem.Block { - // Handling the panic here means that the above write will error and return that error which might be - // acceptable by the end user. - defer panics.HandlePanic() +func PemBlockForKey(priv interface{}, gen ...bool) (*pem.Block, error) { switch k := priv.(type) { case *rsa.PublicKey: if utils.Optional(gen...) { @@ -78,13 +81,13 @@ func PemBlockForKey(priv interface{}, gen ...bool) *pem.Block { if err != nil { panic(err) } - return &pem.Block{Type: "PUBLIC KEY", Bytes: bytes} + return &pem.Block{Type: "PUBLIC KEY", Bytes: bytes}, nil } - return &pem.Block{Type: "RSA PUBLIC KEY", Bytes: x509.MarshalPKCS1PublicKey(k)} + return &pem.Block{Type: "RSA PUBLIC KEY", Bytes: x509.MarshalPKCS1PublicKey(k)}, nil case *rsa.PrivateKey: - return &pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(k)} + return &pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(k)}, nil default: - panic("invalid key") + return nil, errors.ErrInvalid("key") } }