feat: clean the error for starlark output as well (#413)

## Description:
This PR will keep the errors consistent for Starlark execution phase. It
will not show the stack-trace path unless users either pass in
`cli-log-level debug` or they can see the logs with stack-traces in
`kurtosis-cli.log` file. This PR just does the same for starlark as
#369 did for cli.

## Is this change user facing?
YES/NO
<!-- If yes, please add the "user facing" label to the PR -->
<!-- If yes, don't forget to include docs changes where relevant -->

## References (if applicable):
<!-- Add relevant Github Issues, Discord threads, or other helpful
information. -->

cli/cli/command_str_consts/command_str_consts.go

@@ -1,6 +1,7 @@
 package command_str_consts
 
 import (
+	"errors"
 	"os"
 	"path"
 )
@@ -61,5 +62,7 @@ const (
 	PathCmdStr              = "path"
 	VersionCmdStr           = "version"
 	GatewayCmdStr           = "gateway"
-	LspCmdStr               = "lsp"
 )
+
+// TODO: added constant error message here, can we move to another file later.
+var ErrorMessageDueToStarlarkFailure = errors.New("Kurtosis execution threw an error. See output above for more details")

cli/cli/commands/run/run.go

@@ -315,10 +315,6 @@ func run(
 	}
 
 	if errRunningKurtosis != nil {
-		// This error thrown by the APIC is not informative right now as it just tells the user to look at errors
-		// in the above log. For this reason we're ignoring it and returning nil. This is exceptional to not clutter
-		// the CLI output. We should still use stacktrace.Propagate for other errors.
-		// TODO: will do it another way in next PR. Returning the error for now as it broke CI pipeline
 		return errRunningKurtosis
 	}
 
@@ -424,7 +420,10 @@ func readAndPrintResponseLinesUntilClosed(responseLineChan <-chan *kurtosis_core
 		case responseLine, isChanOpen := <-responseLineChan:
 			if !isChanOpen {
 				if !isRunSuccessful {
-					return stacktrace.NewError("Kurtosis execution threw an error. See output above for more details")
+					// This error thrown by the APIC is not informative right now as it just tells the user to look at errors
+					// in the above log. For this reason we're ignoring it and returning nil. This is exceptional to not clutter
+					// the CLI output. We should still use stacktrace.Propagate for other errors.
+					return stacktrace.Propagate(command_str_consts.ErrorMessageDueToStarlarkFailure, "Error occurred while running kurtosis package")
 				}
 				return nil
 			}

cli/cli/helpers/output_printers/kurtosis_instruction_printer.go

@@ -1,6 +1,7 @@
 package output_printers
 
 import (
+	"errors"
 	"fmt"
 	"github.com/bazelbuild/buildtools/build"
 	"github.com/briandowns/spinner"
@@ -124,7 +125,9 @@ func (printer *ExecutionPrinter) PrintKurtosisExecutionResponseLineToStdOut(resp
 		} else if responseLine.GetError().GetValidationError() != nil {
 			errorMsg = fmt.Sprintf("There was an error validating Starlark code \n%v", responseLine.GetError().GetValidationError().GetErrorMessage())
 		} else if responseLine.GetError().GetExecutionError() != nil {
-			errorMsg = fmt.Sprintf("There was an error executing Starlark code \n%v", responseLine.GetError().GetExecutionError().GetErrorMessage())
+			errorMsgWithStackTrace := errors.New(responseLine.GetError().GetExecutionError().GetErrorMessage())
+			cleanedErrorFromStarlark := out.GetErrorMessageToBeDisplayedOnCli(errorMsgWithStackTrace)
+			errorMsg = fmt.Sprintf("There was an error executing Starlark code \n%v", cleanedErrorFromStarlark)
 		}
 		formattedError := formatError(errorMsg)
 		if err := printer.printPersistentLineToStdOut(formattedError); err != nil {

cli/cli/main.go

@@ -7,8 +7,10 @@ package main
 
 import (
 	"errors"
+	"github.com/kurtosis-tech/kurtosis/cli/cli/command_str_consts"
 	"github.com/kurtosis-tech/kurtosis/cli/cli/commands"
 	"github.com/kurtosis-tech/kurtosis/cli/cli/out"
+	"github.com/kurtosis-tech/stacktrace"
 	"github.com/sirupsen/logrus"
 	"os"
 	"strings"
@@ -21,11 +23,8 @@ const (
 	forceColors   = true
 	fullTimestamp = true
 
-	separator                      = "---"
-	removeTrailingSpecialCharacter = "\n"
-	errorNotCreatedFromStacktrace  = 1
-	errorPrefix                    = "Error: "
-	commandNotFound                = "unknown command"
+	errorPrefix     = "Error: "
+	commandNotFound = "unknown command"
 )
 
 func main() {
@@ -49,12 +48,14 @@ func main() {
 	})
 
 	if err := commands.RootCmd.Execute(); err != nil {
-		maybeCleanedError := getErrorMessageToBeDisplayedOnCli(err)
+		if !displayErrorMessageToCli(err) {
+			os.Exit(errorExitCode)
+		}
+
+		maybeCleanedError := out.GetErrorMessageToBeDisplayedOnCli(err)
 		errorMessageFromCli := maybeCleanedError.Error()
-		// cobra uses this method underneath as well to print errors
-		// so just used it directly.
-		commands.RootCmd.PrintErrln(errorPrefix, errorMessageFromCli)
 
+		commands.RootCmd.PrintErrln(errorPrefix, errorMessageFromCli)
 		// if unknown command is entered - display help command
 		if strings.Contains(errorMessageFromCli, commandNotFound) {
 			commands.RootCmd.PrintErrf("Run '%v --help' for usage.\n", commands.RootCmd.CommandPath())
@@ -64,44 +65,7 @@ func main() {
 	os.Exit(successExitCode)
 }
 
-func getErrorMessageToBeDisplayedOnCli(errorWithStacktrace error) error {
-	// if we are running in the debug mode, just return the error with stack-traces back to the client
-	if logrus.GetLevel() == logrus.DebugLevel {
-		return errorWithStacktrace
-	}
-
-	// silently catch the file logger error and print it in the debug mode
-	// users should not worry about this error
-	// downside is that we may lose stack-traces during file logger failures
-	fileLogger, err := out.GetFileLogger()
-	if err != nil {
-		logrus.Warnf("Error occurred while getting the file logger %+v", err)
-	} else {
-		fileLogger.Errorln(errorWithStacktrace.Error())
-	}
-
-	errorMessage := errorWithStacktrace.Error()
-	cleanError := removeFilePathFromErrorMessage(errorMessage)
-	return cleanError
-}
-
-// this method removes the file path from the error
-func removeFilePathFromErrorMessage(errorMessage string) error {
-	errorMessageConvertedInList := strings.Split(errorMessage, separator)
-	// safe to assume that the error was not generated using stacktrace package
-	if len(errorMessageConvertedInList) == errorNotCreatedFromStacktrace {
-		return errors.New(errorMessage)
-	}
-
-	// only the even numbered elements needs to be picked.
-	var cleanErrorList []string
-	for index, line := range errorMessageConvertedInList {
-		if index%2 == 0 {
-			cleanErrorMsgLine := strings.Trim(line, removeTrailingSpecialCharacter)
-			cleanErrorList = append(cleanErrorList, cleanErrorMsgLine)
-		}
-	}
-
-	cleanErrorMessage := strings.Join(cleanErrorList, "")
-	return errors.New(cleanErrorMessage)
+func displayErrorMessageToCli(err error) bool {
+	rootCause := stacktrace.RootCause(err)
+	return !errors.Is(rootCause, command_str_consts.ErrorMessageDueToStarlarkFailure)
 }

cli/cli/out/post_process_error.go

@@ -0,0 +1,59 @@
+package out
+
+import (
+	"errors"
+	"github.com/sirupsen/logrus"
+	"regexp"
+	"strings"
+)
+
+const (
+	lineWithStacktraceRegex       = "^--- at\\s*(.*?):([\\d]*)\\s*\\((.*?)\\)\\s*---$"
+	separator                     = "\n"
+	errorNotCreatedFromStacktrace = 1
+)
+
+var lineWithStacktrace = regexp.MustCompile(lineWithStacktraceRegex)
+
+func GetErrorMessageToBeDisplayedOnCli(errorWithStacktrace error) error {
+	// if we are running in the debug mode, just return the error with stack-traces back to the client
+	if logrus.GetLevel() == logrus.DebugLevel {
+		return errorWithStacktrace
+	}
+
+	// silently catch the file logger error and print it in the debug mode
+	// users should not worry about this error
+	// downside is that we may lose stack-traces during file logger failures
+	fileLogger, err := GetFileLogger()
+	if err != nil {
+		logrus.Warnf("Error occurred while getting the file logger %+v", err)
+	} else {
+		fileLogger.Errorln(errorWithStacktrace.Error())
+	}
+
+	errorMessage := errorWithStacktrace.Error()
+	cleanError := removeFilePathFromErrorMessage(errorMessage)
+	return cleanError
+}
+
+// this method removes the file path from the error
+func removeFilePathFromErrorMessage(errorMessage string) error {
+	errorMessageConvertedInList := strings.Split(errorMessage, separator)
+	// safe to assume that the error was not generated using stacktrace package
+	if len(errorMessageConvertedInList) == errorNotCreatedFromStacktrace {
+		return errors.New(errorMessage)
+	}
+
+	// only the even numbered elements needs to be picked.
+	var cleanErrorList []string
+	for _, line := range errorMessageConvertedInList {
+		// this only cleans spaces for the lines that contains the stack-trace information
+		cleanLine := strings.TrimSpace(line)
+		if !lineWithStacktrace.MatchString(cleanLine) {
+			cleanErrorList = append(cleanErrorList, cleanLine)
+		}
+	}
+
+	cleanErrorMessage := strings.Join(cleanErrorList, "\n")
+	return errors.New(cleanErrorMessage)
+}

cli/cli/main_test.go → cli/cli/out/post_process_error_test.go

@@ -1,4 +1,4 @@
-package main
+package out
 
 import (
 	"github.com/kurtosis-tech/stacktrace"
@@ -9,12 +9,12 @@ import (
 func TestRemoveFilePathFromErrorMessage(t *testing.T) {
 	stacktraceErr := createDummyStackTraceWithNonEmptyMsg()
 	errorClean := removeFilePathFromErrorMessage(stacktraceErr.Error())
-	expectedValue := "this is propagated error\n Caused by: Error: this is base error\n "
+	expectedValue := "this is propagated error\nCaused by: Error: this is base error"
 	require.Equal(t, expectedValue, errorClean.Error())
 
 	stacktraceErrEmpty := createDummyStackTraceWithEmptyMsg()
 	errorClean = removeFilePathFromErrorMessage(stacktraceErrEmpty.Error())
-	expectedValue = " Caused by: Error: this is base error\n "
+	expectedValue = "Caused by: Error: this is base error"
 	require.Equal(t, expectedValue, errorClean.Error())
 }