Improve errext helper names and behaviors

cmd/cloud.go

@@ -327,7 +327,7 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth
 
 			if testProgress == nil {
 				//nolint:stylecheck,golint
-				return errext.WithExitCode(errors.New("Test progress error"), exitcodes.CloudFailedToGetProgress)
+				return errext.WithExitCodeIfNone(errors.New("Test progress error"), exitcodes.CloudFailedToGetProgress)
 			}
 
 			valueColor := getColor(noColor || !stdoutTTY, color.FgCyan)
@@ -336,7 +336,7 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth
 			if testProgress.ResultStatus == cloudapi.ResultStatusFailed {
 				// TODO: use different exit codes for failed thresholds vs failed test (e.g. aborted by system/limit)
 				//nolint:stylecheck,golint
-				return errext.WithExitCode(errors.New("The test has failed"), exitcodes.CloudTestRunFailed)
+				return errext.WithExitCodeIfNone(errors.New("The test has failed"), exitcodes.CloudTestRunFailed)
 			}
 
 			return nil

cmd/config.go

@@ -171,7 +171,7 @@ func readEnvConfig() (Config, error) {
 // TODO: add better validation, more explicit default values and improve consistency between formats
 // TODO: accumulate all errors and differentiate between the layers?
 func getConsolidatedConfig(fs afero.Fs, cliConf Config, runner lib.Runner) (conf Config, err error) {
-	// TODO: use errext.WithExitCode(err, exitcodes.InvalidConfig) where it makes sense?
+	// TODO: use errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) where it makes sense?
 
 	fileConf, _, err := readDiskConfig(fs)
 	if err != nil {
@@ -232,7 +232,7 @@ func deriveAndValidateConfig(conf Config, isExecutable func(string) bool) (resul
 	if err == nil {
 		err = validateConfig(result, isExecutable)
 	}
-	return result, errext.WithExitCode(err, exitcodes.InvalidConfig)
+	return result, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig)
 }
 
 func validateConfig(conf Config, isExecutable func(string) bool) error {

cmd/run.go

@@ -248,7 +248,7 @@ a commandline interface for interacting with it.`,
 			engineRun, engineWait, err := engine.Init(globalCtx, runCtx)
 			if err != nil {
 				// Add a generic engine exit code if we don't have a more specific one
-				return errext.WithExitCode(err, exitcodes.GenericEngine)
+				return errext.WithExitCodeIfNone(err, exitcodes.GenericEngine)
 			}
 
 			// Init has passed successfully, so unless disabled, make sure we send a
@@ -271,7 +271,7 @@ a commandline interface for interacting with it.`,
 			// Start the test run
 			initBar.Modify(pb.WithConstProgress(0, "Starting test..."))
 			if err := engineRun(); err != nil {
-				return errext.WithExitCode(err, exitcodes.GenericEngine)
+				return errext.WithExitCodeIfNone(err, exitcodes.GenericEngine)
 			}
 			runCancel()
 			logger.Debug("Engine run terminated cleanly")
@@ -321,7 +321,7 @@ a commandline interface for interacting with it.`,
 			engineWait()
 			logger.Debug("Everything has finished, exiting k6!")
 			if engine.IsTainted() {
-				return errext.WithExitCode(errors.New("some thresholds have failed"), exitcodes.ThresholdsHaveFailed)
+				return errext.WithExitCodeIfNone(errors.New("some thresholds have failed"), exitcodes.ThresholdsHaveFailed)
 			}
 			return nil
 		},

errext/errext_test.go

@@ -48,7 +48,7 @@ func TestErrextHelpers(t *testing.T) {
 
 	const testExitCode ExitCode = 13
 	assert.Nil(t, WithHint(nil, "test hint"))
-	assert.Nil(t, WithExitCode(nil, testExitCode))
+	assert.Nil(t, WithExitCodeIfNone(nil, testExitCode))
 
 	errBase := errors.New("base error")
 	errBaseWithHint := WithHint(errBase, "test hint")
@@ -59,11 +59,11 @@ func TestErrextHelpers(t *testing.T) {
 	errWrapperWithHints := fmt.Errorf("wrapper error: %w", errBaseWithTwoHints)
 	assertHasHint(t, errWrapperWithHints, "better hint (test hint)")
 
-	errWithExitCode := WithExitCode(errWrapperWithHints, testExitCode)
+	errWithExitCode := WithExitCodeIfNone(errWrapperWithHints, testExitCode)
 	assertHasHint(t, errWithExitCode, "better hint (test hint)")
 	assertHasExitCode(t, errWithExitCode, testExitCode)
 
-	errWithExitCodeAgain := WithExitCode(errWithExitCode, ExitCode(27))
+	errWithExitCodeAgain := WithExitCodeIfNone(errWithExitCode, ExitCode(27))
 	assertHasHint(t, errWithExitCodeAgain, "better hint (test hint)")
 	assertHasExitCode(t, errWithExitCodeAgain, testExitCode)
 

errext/exit_code.go

@@ -33,11 +33,11 @@ type HasExitCode interface {
 	ExitCode() ExitCode
 }
 
-// WithExitCode can attach an exit code to the given error, if it doesn't have
-// one already. It won't do anything if the error already had an exit code
+// WithExitCodeIfNone can attach an exit code to the given error, if it doesn't
+// have one already. It won't do anything if the error already had an exit code
 // attached. Similarly, if there is no error (i.e. the given error is nil), it
 // also won't do anything.
-func WithExitCode(err error, exitCode ExitCode) error {
+func WithExitCodeIfNone(err error, exitCode ExitCode) error {
 	if err == nil {
 		// No error, do nothing
 		return nil

errext/hint.go

@@ -36,13 +36,7 @@ type HasHint interface {
 // "new hint (old hint)".
 func WithHint(err error, hint string) error {
 	if err == nil {
-		// No error, do nothing
-		return nil
-	}
-	var oldhint HasHint
-	if errors.As(err, &oldhint) {
-		// The given error already had a hint, wrap it
-		hint = hint + " (" + oldhint.Hint() + ")"
+		return nil // No error, do nothing
 	}
 	return withHint{err, hint}
 }
@@ -57,7 +51,14 @@ func (wh withHint) Unwrap() error {
 }
 
 func (wh withHint) Hint() string {
-	return wh.hint
+	hint := wh.hint
+	var oldhint HasHint
+	if errors.As(wh.error, &oldhint) {
+		// The given error already had a hint, wrap it
+		hint = hint + " (" + oldhint.Hint() + ")"
+	}
+
+	return hint
 }
 
 var _ HasHint = withHint{}

js/runner.go

@@ -741,7 +741,11 @@ type scriptException struct {
 	inner *goja.Exception
 }
 
-var _ errext.Exception = &scriptException{}
+var (
+	_ errext.Exception   = &scriptException{}
+	_ errext.HasExitCode = &scriptException{}
+	_ errext.HasHint     = &scriptException{}
+)
 
 func (s *scriptException) Error() string {
 	// this calls String instead of error so that by default if it's printed to print the stacktrace