fix: ensure nested legacycli invocations forward errors

cliv2/cmd/cliv2/errorhandling.go

@@ -2,11 +2,13 @@ package main
 
 import (
 	"errors"
+	"iter"
 	"os/exec"
 
 	"github.com/snyk/error-catalog-golang-public/cli"
 	"github.com/snyk/error-catalog-golang-public/snyk_errors"
 
+	"github.com/snyk/cli/cliv2/internal/cliv2"
 	cli_errors "github.com/snyk/cli/cliv2/internal/errors"
 )
 
@@ -32,3 +34,70 @@ func decorateError(err error) error {
 	}
 	return err
 }
+
+// getErrorMessage returns the appropriate error message for the specified error. Defaults to the standard error message method,
+// but if the error matches the Error Catalog model, the returned value will become the detail field.
+func getErrorMessage(err error) string {
+	message := err.Error()
+	snykErr := snyk_errors.Error{}
+	if errors.As(err, &snykErr) {
+		message = snykErr.Detail
+	}
+
+	return message
+}
+
+// errorHasBeenShown return whether the error was already presented by the Typescript CLI or not.
+// This will iterate through the error chain to find if any of the errors in the chain has been shown.
+func errorHasBeenShown(err error) bool {
+	for err := range iterErrorChain(err) {
+		snykErr := snyk_errors.Error{}
+		if errors.As(err, &snykErr) {
+			wasDisplayed, ok := snykErr.Meta[cliv2.ERROR_HAS_BEEN_DISPLAYED].(bool)
+			if !ok {
+				continue
+			}
+
+			// we stop checking the chain if we find an error that was presented already.
+			if wasDisplayed {
+				return true
+			}
+		}
+	}
+
+	// none of the errors in the chain were presented.
+	return false
+}
+
+// IterErrorChain returns an iterator with all the errors from the error parameter, including the initial error.
+// Taken from this proposal: https://github.com/golang/go/issues/66455
+// Eg: errA -> errB -> errC will yield an iterator with the following errors:
+// ["errA -> errB -> errC", "errA", "errB", "errC"]
+func iterErrorChain(err error) iter.Seq[error] {
+	return func(yield func(error) bool) {
+		yieldAll(err, yield)
+	}
+}
+
+// yieldAll is the generator function that walks the error chain.
+func yieldAll(err error, yield func(error) bool) bool {
+	for err != nil {
+		if !yield(err) {
+			return false
+		}
+		switch x := err.(type) {
+		case interface{ Unwrap() error }:
+			err = x.Unwrap()
+		case interface{ Unwrap() []error }:
+			for _, err := range x.Unwrap() {
+				if !yieldAll(err, yield) {
+					return false
+				}
+			}
+			return true
+		default:
+			return true
+		}
+	}
+	return true
+}

cliv2/cmd/cliv2/errorhandling_test.go

@@ -2,14 +2,17 @@ package main
 
 import (
 	"errors"
+	"fmt"
 	"os"
 	"os/exec"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 
 	"github.com/snyk/error-catalog-golang-public/cli"
+	"github.com/snyk/error-catalog-golang-public/snyk_errors"
 
+	"github.com/snyk/cli/cliv2/internal/cliv2"
 	cli_errors "github.com/snyk/cli/cliv2/internal/errors"
 )
 
@@ -45,3 +48,103 @@ func Test_decorateError(t *testing.T) {
 		assert.ErrorAs(t, actualErrr, &expectedError)
 	})
 }
+
+func Test_iterErrorChain(t *testing.T) {
+	t.Run("wrapping errors.Join", func(t *testing.T) {
+		err1 := errors.New("first error")
+		err2 := fmt.Errorf("second error")
+		joined := errors.Join(err1, err2)
+
+		errors := []error{}
+		for err := range iterErrorChain(joined) {
+			errors = append(errors, err)
+		}
+
+		assert.Contains(t, errors, joined)
+		assert.Contains(t, errors, err1)
+		assert.Contains(t, errors, err2)
+	})
+
+	t.Run("wrapping using fmt.Errorf", func(t *testing.T) {
+		err1 := errors.New("first error")
+		err2 := fmt.Errorf("second error")
+		wrapped := fmt.Errorf("wrapped: %w %w", err1, err2)
+
+		errors := []error{}
+		for err := range iterErrorChain(wrapped) {
+			errors = append(errors, err)
+		}
+
+		assert.Contains(t, errors, wrapped)
+		assert.Contains(t, errors, err1)
+		assert.Contains(t, errors, err2)
+	})
+
+	t.Run("combined wrapping", func(t *testing.T) {
+		err1 := errors.New("first error")
+		err2 := fmt.Errorf("second error")
+		joined := errors.Join(err1, err2)
+
+		cause := errors.New("cause")
+		snykErr := snyk_errors.Error{
+			Title: "error struct",
+			Cause: cause,
+		}
+
+		wrapped := fmt.Errorf("wrapped: %w %w", snykErr, joined)
+
+		errors := []error{}
+		for err := range iterErrorChain(wrapped) {
+			errors = append(errors, err)
+		}
+
+		assert.Contains(t, errors, wrapped)
+		assert.Contains(t, errors, snykErr)
+		assert.Contains(t, errors, cause)
+		assert.Contains(t, errors, joined)
+		assert.Contains(t, errors, err2)
+		assert.Contains(t, errors, err1)
+	})
+}
+
+func Test_errorHasBeenShown(t *testing.T) {
+	t.Run("has been displayed", func(t *testing.T) {
+		err := snyk_errors.Error{
+			Meta: map[string]any{
+				cliv2.ERROR_HAS_BEEN_DISPLAYED: true,
+			},
+		}
+
+		hasBeenShown := errorHasBeenShown(err)
+		assert.Equal(t, hasBeenShown, true)
+	})
+
+	t.Run("unset value", func(t *testing.T) {
+		err := snyk_errors.Error{
+			Meta: map[string]any{},
+		}
+
+		hasBeenShown := errorHasBeenShown(err)
+		assert.Equal(t, hasBeenShown, false)
+	})
+
+	t.Run("multiple errors in chain", func(t *testing.T) {
+		first := snyk_errors.Error{
+			Meta: map[string]any{
+				cliv2.ERROR_HAS_BEEN_DISPLAYED: false,
+			},
+		}
+		second := snyk_errors.Error{
+			Meta: map[string]any{},
+		}
+		third := snyk_errors.Error{
+			Meta: map[string]any{
+				cliv2.ERROR_HAS_BEEN_DISPLAYED: true,
+			},
+		}
+		chain := fmt.Errorf("exit error: %w", errors.Join(first, second, third))
+
+		hasBeenShown := errorHasBeenShown(chain)
+		assert.Equal(t, hasBeenShown, true)
+	})
+}

cliv2/cmd/cliv2/main.go

@@ -457,14 +457,16 @@ func displayError(err error, userInterface ui.UserInterface, config configuratio
 	if err != nil {
 		_, isExitError := err.(*exec.ExitError)
 		_, isErrorWithCode := err.(*cli_errors.ErrorWithExitCode)
-		if isExitError || isErrorWithCode {
+		if isExitError || isErrorWithCode || errorHasBeenShown(err) {
 			return
 		}
 
 		if config.GetBool(output_workflow.OUTPUT_CONFIG_KEY_JSON) {
+			message := getErrorMessage(err)
+
 			jsonError := JsonErrorStruct{
 				Ok:       false,
-				ErrorMsg: err.Error(),
+				ErrorMsg: message,
 				Path:     globalConfiguration.GetString(configuration.INPUT_DIRECTORY),
 			}
 
@@ -543,6 +545,9 @@ func MainWithErrorCode() (int, []error) {
 	outputWorkflow, _ := globalEngine.GetWorkflow(localworkflows.WORKFLOWID_OUTPUT_WORKFLOW)
 	outputFlags := workflow.FlagsetFromConfigurationOptions(outputWorkflow.GetConfigurationOptions())
 	rootCommand.PersistentFlags().AddFlagSet(outputFlags)
+	// add output flags as persistent flags
+	_ = rootCommand.ParseFlags(os.Args)
+	globalConfiguration.AddFlagSet(rootCommand.LocalFlags())
 
 	// add workflows as commands
 	createCommandsForWorkflows(rootCommand, globalEngine)

cliv2/internal/cliv2/cliv2.go

@@ -25,6 +25,7 @@ import (
 	"github.com/snyk/error-catalog-golang-public/snyk_errors"
 	"github.com/snyk/go-application-framework/pkg/configuration"
 	"github.com/snyk/go-application-framework/pkg/instrumentation"
+	"github.com/snyk/go-application-framework/pkg/local_workflows/output_workflow"
 	"github.com/snyk/go-application-framework/pkg/runtimeinfo"
 	"github.com/snyk/go-application-framework/pkg/utils"
 
@@ -61,7 +62,10 @@ const (
 	V2_ABOUT   Handler = iota
 )
 
-const configKeyErrFile = "INTERNAL_ERR_FILE_PATH"
+const (
+	configKeyErrFile         = "INTERNAL_ERR_FILE_PATH"
+	ERROR_HAS_BEEN_DISPLAYED = "hasBeenDisplayed"
+)
 
 func NewCLIv2(config configuration.Configuration, debugLogger *log.Logger, ri runtimeinfo.RuntimeInfo) (*CLI, error) {
 	cacheDirectory := config.GetString(configuration.CACHE_PATH)
@@ -482,9 +486,12 @@ func (c *CLI) getErrorFromFile(errFilePath string) (data error, err error) {
 	}
 
 	if len(jsonErrors) != 0 {
+		hasBeenDisplayed := GetErrorDisplayStatus(c.globalConfig)
+
 		errs := make([]error, len(jsonErrors)+1)
 		for _, jerr := range jsonErrors {
 			jerr.Meta["orign"] = "Typescript-CLI"
+			jerr.Meta[ERROR_HAS_BEEN_DISPLAYED] = hasBeenDisplayed
 			errs = append(errs, jerr)
 		}
 
@@ -562,3 +569,17 @@ func DetermineInputDirectory(args []string) string {
 	}
 	return ""
 }
+
+// GetErrorDisplayStatus computes whether the IPC error was displayed by the TS CLI or not. It accounts for
+// the usage of STDIO and the presence of the JSON flag when the Legacy CLI was invoked.
+func GetErrorDisplayStatus(config configuration.Configuration) bool {
+	useSTDIO := config.GetBool(configuration.WORKFLOW_USE_STDIO)
+	jsonEnabled := config.GetBool(output_workflow.OUTPUT_CONFIG_KEY_JSON)
+
+	hasBeenDisplayed := false
+	if useSTDIO && jsonEnabled {
+		hasBeenDisplayed = true
+	}
+
+	return hasBeenDisplayed
+}

cliv2/internal/cliv2/cliv2_test.go

@@ -3,6 +3,7 @@ package cliv2_test
 import (
 	"context"
 	"errors"
+	"fmt"
 	"io"
 	"log"
 	"os"
@@ -15,6 +16,7 @@ import (
 
 	"github.com/snyk/go-application-framework/pkg/app"
 	"github.com/snyk/go-application-framework/pkg/configuration"
+	"github.com/snyk/go-application-framework/pkg/local_workflows/output_workflow"
 	"github.com/snyk/go-application-framework/pkg/runtimeinfo"
 	"github.com/snyk/go-application-framework/pkg/utils"
 
@@ -537,3 +539,51 @@ func Test_determineInputDirectory(t *testing.T) {
 		assert.Equal(t, expected, actual)
 	})
 }
+
+func Test_GetErrorDisplayStatus(t *testing.T) {
+	// Decision table:
+	//  stdin |  json  | hasBeenDisplayed
+	// ---------------------------------
+	//  true  |  true  |    true
+	//  true  |  false |    false
+	//  false |    *   |    false
+
+	tests := []struct {
+		stdin    bool
+		json     bool
+		expected bool
+	}{
+		{
+			stdin:    true,
+			json:     true,
+			expected: true,
+		},
+		{
+			stdin:    true,
+			json:     false,
+			expected: false,
+		},
+		{
+			stdin:    false,
+			json:     true,
+			expected: false,
+		},
+		{
+			stdin:    false,
+			json:     false,
+			expected: false,
+		},
+	}
+
+	for _, tc := range tests {
+		testName := fmt.Sprintf("%t + %t = %t", tc.stdin, tc.json, tc.expected)
+		t.Run(testName, func(t *testing.T) {
+			config := configuration.NewWithOpts(configuration.WithAutomaticEnv())
+			config.Set(configuration.WORKFLOW_USE_STDIO, tc.stdin)
+			config.Set(output_workflow.OUTPUT_CONFIG_KEY_JSON, tc.json)
+
+			hasBeenDisplayed := cliv2.GetErrorDisplayStatus(config)
+			assert.Equal(t, hasBeenDisplayed, tc.expected)
+		})
+	}
+}