Merge remote-tracking branch 'upstream/main' into add-nested-archive-cataloging

Signed-off-by: Keith Zantow <keith.zantow@anchore.com>

internal/task/archive_tasks.go

@@ -302,7 +302,7 @@ func (c *archiveCataloger) runSubPipeline(ctx context.Context, resolver file.Res
 	scratch := &sbom.SBOM{Artifacts: sbom.Artifacts{Packages: pkg.NewCollection()}}
 	scratchBuilder := sbomsync.NewBuilder(scratch)
 	for _, t := range c.subPipeline {
-		err := runTaskSafely(ctx, t, resolver, scratchBuilder)
+		err := runSubTaskSafely(ctx, t, resolver, scratchBuilder)
 		if err == nil {
 			continue
 		}
@@ -317,6 +317,18 @@ func (c *archiveCataloger) runSubPipeline(ctx context.Context, resolver file.Res
 	return scratch
 }
 
+// runSubTaskSafely is runTaskSafely without the task name in a panic's error, since appendUnknowns
+// already prefixes it.
+func runSubTaskSafely(ctx context.Context, t Task, resolver file.Resolver, builder sbomsync.Builder) (err error) {
+	defer func() {
+		if r := recover(); r != nil {
+			log.WithFields("task", t.Name()).Debugf("archive sub-cataloger task panic stack:\n%s", debug.Stack())
+			err = fmt.Errorf("panic: %v", r)
+		}
+	}()
+	return t.Execute(ctx, resolver, builder)
+}
+
 // archiveCandidate is one file the walk will try to open.
 type archiveCandidate struct {
 	location file.Location

internal/task/archive_tasks_test.go

@@ -577,7 +577,7 @@ func Test_archiveCataloger_subPipelineFailureIsRecordedAsAnUnknown(t *testing.T)
 		reasons, ok := s.Artifacts.Unknowns[file.Coordinates{RealPath: "/outer.zip"}]
 		require.True(t, ok)
 		require.Len(t, reasons, 1)
-		assert.Contains(t, reasons[0], "panicking-cataloger: nope")
+		assert.Equal(t, "panicking-cataloger: panic: nope", reasons[0])
 	})
 }
 

internal/task/executor.go

@@ -51,7 +51,8 @@ func runTaskSafely(ctx context.Context, t Task, resolver file.Resolver, s sbomsy
 	// handle individual cataloger panics
 	defer func() {
 		if e := recover(); e != nil {
-			err = fmt.Errorf("%v at:\n%s", e, string(debug.Stack()))
+			log.WithFields("task", t.Name()).Debugf("task panic stack:\n%s", debug.Stack())
+			err = fmt.Errorf("panic in task %q: %v", t.Name(), e)
 		}
 	}()
 

internal/task/executor_test.go

@@ -8,8 +8,12 @@ import (
 	"github.com/wagoodman/go-progress"
 
 	"github.com/anchore/syft/internal/sbomsync"
+	"github.com/anchore/syft/syft/artifact"
 	"github.com/anchore/syft/syft/event/monitor"
 	"github.com/anchore/syft/syft/file"
+	"github.com/anchore/syft/syft/pkg"
+	"github.com/anchore/syft/syft/pkg/cataloger/generic"
+	"github.com/anchore/syft/syft/sbom"
 )
 
 func Test_TaskExecutor_PanicHandling(t *testing.T) {
@@ -21,6 +25,20 @@ func Test_TaskExecutor_PanicHandling(t *testing.T) {
 		Manual: progress.NewManual(-1),
 	})
 
-	require.Error(t, err)
-	require.Contains(t, err.Error(), "executor_test.go")
+	require.EqualError(t, err, `panic in task "panicking-cataloger": something bad happened`)
+}
+
+func Test_RunTask_GenericParserPanicBecomesUnknown(t *testing.T) {
+	c := generic.NewCataloger("panicking-cataloger").
+		WithParserByPath(func(context.Context, file.Resolver, *generic.Environment, file.LocationReadCloser) ([]pkg.Package, []artifact.Relationship, error) {
+			panic("boom")
+		}, "executor_test.go")
+	s := &sbom.SBOM{}
+
+	err := RunTask(context.Background(), NewPackageTask(DefaultCatalogingFactoryConfig(), c), file.NewMockResolverForPaths("executor_test.go"), sbomsync.NewBuilder(s), &monitor.TaskProgress{
+		Manual: progress.NewManual(-1),
+	})
+
+	require.NoError(t, err)
+	require.Len(t, s.Artifacts.Unknowns, 1)
 }