#105 - Panic Handlers

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,9 @@ 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
 	valid := true
@@ -365,6 +369,9 @@ 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)

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,8 @@ 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
 	}

pkg/cobrautils/links.go

@@ -9,6 +9,8 @@ import (
 	"strings"
 
 	"github.com/spf13/cobra"
+
+	"github.com/open-component-model/ocm/pkg/utils/panics"
 )
 
 func LinkForCmd(cmd *cobra.Command) string {
@@ -32,6 +34,8 @@ 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, "<CMD>")

pkg/contexts/datacontext/util.go

@@ -26,7 +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) {
+func SetupElement[T ElementCopyable[T]](mode BuilderMode, target *T, create ElementCreator[T], def T) error {
 	var zero T
 	if *target == zero {
 		switch mode {
@@ -41,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
 }

pkg/contexts/oci/attrs/cacheattr/attr.go

@@ -51,7 +51,7 @@ func (a AttributeType) Decode(data []byte, unmarshaller runtime.Unmarshaler) (in
 		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:]
 		}

pkg/contexts/oci/cpi/flavor_artifact.go

@@ -77,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)

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"
@@ -31,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)

pkg/contexts/oci/cpi/flavor_manifest.go

@@ -6,6 +6,7 @@ package cpi
 
 import (
 	"compress/gzip"
+	"fmt"
 
 	"github.com/opencontainers/go-digest"
 
@@ -32,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)

pkg/contexts/oci/cpi/support/flavor_artifact.go

@@ -49,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)
 }

pkg/contexts/oci/repositories/ocireg/blobs.go

@@ -86,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
 }

pkg/contexts/oci/repositories/ocireg/namespace.go

@@ -120,15 +120,23 @@ 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
 }
 
 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
 			}
 		}

pkg/contexts/ocm/internal/builder.go

@@ -189,7 +189,7 @@ func (d *delegatingDecoder) Decode(data []byte, unmarshaler runtime.Unmarshaler)
 	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
 	}

pkg/contexts/ocm/utils.go

@@ -35,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)
 		}
 	}
 

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,9 @@ 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:
 		if utils.Optional(gen...) {

pkg/utils/panics/panics.go

@@ -0,0 +1,51 @@
+// 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"
+)
+
+var ReallyCrash = true
+
+type PanicHandler func(interface{})
+
+var PanicHandlers = []PanicHandler{logHandler}
+
+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)
+		}
+	}
+}
+
+func RegisterPanicHandler(handler PanicHandler) {
+	PanicHandlers = append(PanicHandlers, handler)
+}
+
+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
+	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))
+	}
+}

pkg/utils/utils.go

@@ -294,7 +294,7 @@ 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 {
 	if err != nil {
 		panic(fmt.Errorf("expected a %T, but got error %w", o, err))