Add new AbortReason type, use old RunStatus type only for k6 cloud code

RunStatus is very specific to the k6 cloud and doesn't map perfectly to how k6 OSS sees the causes of prematurely stopped tests. So, this commit restricts the usage of RunStatus only to the cloudapi/ package and to the `k6 cloud` command handling in `cmd/cloud.go`.

It does that by introducing a new type, `errext.AbortReason`, and a way to attach this type to test run errors. This allows us a cleaner error propagation to the outputs, and every output can map these generic values to its own internal ones however it wants. It is not necessary for that mapping to be exactly 1:1 and, indeed, the `errext.AbortReason`:`cloudapi.RunStatus` mapping in cloudapi/ is not 1:1.

api/v1/setup_teardown_routes_test.go

@@ -142,7 +142,7 @@ func TestSetupData(t *testing.T) {
 			require.NoError(t, err)
 
 			require.NoError(t, engine.OutputManager.StartOutputs())
-			defer engine.OutputManager.StopOutputs()
+			defer engine.OutputManager.StopOutputs(nil)
 
 			globalCtx, globalCancel := context.WithCancel(context.Background())
 			runCtx, runCancel := context.WithCancel(globalCtx)

api/v1/status_routes_test.go

@@ -116,7 +116,7 @@ func TestPatchStatus(t *testing.T) {
 			require.NoError(t, err)
 
 			require.NoError(t, engine.OutputManager.StartOutputs())
-			defer engine.OutputManager.StopOutputs()
+			defer engine.OutputManager.StopOutputs(nil)
 
 			ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
 			run, wait, err := engine.Init(ctx, ctx)

cmd/cloud.go

@@ -62,6 +62,7 @@ func (c *cmdCloud) preRun(cmd *cobra.Command, args []string) error {
 }
 
 // TODO: split apart some more
+//
 //nolint:funlen,gocognit,cyclop
 func (c *cmdCloud) run(cmd *cobra.Command, args []string) error {
 	printBanner(c.gs)

cmd/integration_test.go

@@ -1338,7 +1338,7 @@ func TestMinIterationDuration(t *testing.T) {
 		import { Counter } from 'k6/metrics';
 
 		export let options = {
-			minIterationDuration: '5s',
+			minIterationDuration: '7s',
 			setupTimeout: '2s',
 			teardownTimeout: '2s',
 			thresholds: {
@@ -1358,9 +1358,9 @@ func TestMinIterationDuration(t *testing.T) {
 	start := time.Now()
 	newRootCommand(ts.globalState).execute()
 	elapsed := time.Since(start)
-	assert.Greater(t, elapsed, 5*time.Second, "expected more time to have passed because of minIterationDuration")
+	assert.Greater(t, elapsed, 7*time.Second, "expected more time to have passed because of minIterationDuration")
 	assert.Less(
-		t, elapsed, 10*time.Second,
+		t, elapsed, 14*time.Second,
 		"expected less time to have passed because minIterationDuration should not affect setup() and teardown() ",
 	)
 

cmd/run.go

@@ -36,7 +36,7 @@ type cmdRun struct {
 // TODO: split apart some more
 //
 //nolint:funlen,gocognit,gocyclo,cyclop
-func (c *cmdRun) run(cmd *cobra.Command, args []string) error {
+func (c *cmdRun) run(cmd *cobra.Command, args []string) (err error) {
 	printBanner(c.gs)
 
 	test, err := loadAndConfigureTest(c.gs, cmd, args, getConfig)
@@ -156,7 +156,9 @@ func (c *cmdRun) run(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	defer engine.OutputManager.StopOutputs()
+	defer func() {
+		engine.OutputManager.StopOutputs(err)
+	}()
 
 	printExecutionDescription(
 		c.gs, "local", args[0], "", conf, execScheduler.GetState().ExecutionTuple, executionPlan, outputs,
@@ -271,7 +273,9 @@ func (c *cmdRun) run(cmd *cobra.Command, args []string) error {
 		} else {
 			logger.Error("some thresholds have failed") // log this, even if there was already a previous error
 		}
-		err = errext.WithExitCodeIfNone(err, exitcodes.ThresholdsHaveFailed)
+		err = errext.WithAbortReasonIfNone(
+			errext.WithExitCodeIfNone(err, exitcodes.ThresholdsHaveFailed), errext.AbortedByThresholdsAfterTestEnd,
+		)
 	}
 	return err
 }

core/engine.go

@@ -8,7 +8,6 @@ import (
 
 	"github.com/sirupsen/logrus"
 
-	"go.k6.io/k6/cloudapi"
 	"go.k6.io/k6/errext"
 	"go.k6.io/k6/errext/exitcodes"
 	"go.k6.io/k6/lib"
@@ -108,7 +107,6 @@ func (e *Engine) Init(globalCtx, runCtx context.Context) (run func() error, wait
 	// TODO: if we ever need metrics processing in the init context, we can move
 	// this below the other components... or even start them concurrently?
 	if err := e.ExecutionScheduler.Init(runCtx, e.Samples); err != nil {
-		e.setRunStatusFromError(err)
 		return nil, nil, err
 	}
 
@@ -148,18 +146,6 @@ func (e *Engine) Init(globalCtx, runCtx context.Context) (run func() error, wait
 	return runFn, waitFn, nil
 }
 
-func (e *Engine) setRunStatusFromError(err error) {
-	var serr errext.Exception
-	switch {
-	case errors.As(err, &serr):
-		e.OutputManager.SetRunStatus(cloudapi.RunStatusAbortedScriptError)
-	case errext.IsInterruptError(err):
-		e.OutputManager.SetRunStatus(cloudapi.RunStatusAbortedUser)
-	default:
-		e.OutputManager.SetRunStatus(cloudapi.RunStatusAbortedSystem)
-	}
-}
-
 // This starts a bunch of goroutines to process metrics, thresholds, and set the
 // test run status when it ends. It returns a function that can be used after
 // the provided context is called, to wait for the complete winding down of all
@@ -196,27 +182,31 @@ func (e *Engine) startBackgroundProcesses(
 		case err = <-execRunResult:
 			if err != nil {
 				e.logger.WithError(err).Debug("run: execution scheduler returned an error")
-				e.setRunStatusFromError(err)
 			} else {
 				e.logger.Debug("run: execution scheduler finished nominally")
-				e.OutputManager.SetRunStatus(cloudapi.RunStatusFinished)
 			}
 			// do nothing, return the same err value we got from the Run()
 			// ExecutionScheduler result, we just set the run_status based on it
 		case <-runCtx.Done():
 			e.logger.Debug("run: context expired; exiting...")
-			e.OutputManager.SetRunStatus(cloudapi.RunStatusAbortedUser)
-			err = errext.WithExitCodeIfNone(errors.New("test run aborted by signal"), exitcodes.ExternalAbort)
+			err = errext.WithAbortReasonIfNone(
+				errext.WithExitCodeIfNone(errors.New("test run aborted by signal"), exitcodes.ExternalAbort),
+				errext.AbortedByUser,
+			)
 		case <-e.stopChan:
 			runSubCancel()
 			e.logger.Debug("run: stopped by user via REST API; exiting...")
-			e.OutputManager.SetRunStatus(cloudapi.RunStatusAbortedUser)
-			err = errext.WithExitCodeIfNone(errors.New("test run stopped from the REST API"), exitcodes.ScriptStoppedFromRESTAPI)
+			err = errext.WithAbortReasonIfNone(
+				errext.WithExitCodeIfNone(errors.New("test run stopped from the REST API"), exitcodes.ScriptStoppedFromRESTAPI),
+				errext.AbortedByUser,
+			)
 		case <-thresholdAbortChan:
 			e.logger.Debug("run: stopped by thresholds; exiting...")
 			runSubCancel()
-			e.OutputManager.SetRunStatus(cloudapi.RunStatusAbortedThreshold)
-			err = errext.WithExitCodeIfNone(errors.New("test run aborted by failed thresholds"), exitcodes.ThresholdsHaveFailed)
+			err = errext.WithAbortReasonIfNone(
+				errext.WithExitCodeIfNone(errors.New("test run aborted by failed thresholds"), exitcodes.ThresholdsHaveFailed),
+				errext.AbortedByThreshold,
+			)
 		}
 	}()
 

core/engine_test.go

@@ -109,7 +109,7 @@ func newTestEngineWithTestPreInitState( //nolint:golint
 			test.runCancel()
 			globalCancel()
 			waitFn()
-			engine.OutputManager.StopOutputs()
+			engine.OutputManager.StopOutputs(nil)
 		},
 		piState: piState,
 	}
@@ -1333,7 +1333,7 @@ func TestActiveVUsCount(t *testing.T) {
 		require.NoError(t, err)
 		cancel()
 		waitFn()
-		engine.OutputManager.StopOutputs()
+		engine.OutputManager.StopOutputs(nil)
 		require.False(t, engine.IsTainted())
 	}
 

errext/abort_reason.go

@@ -0,0 +1,54 @@
+package errext
+
+import "errors"
+
+// AbortReason is used to signal to outputs what type of error caused the test
+// run to be stopped prematurely.
+type AbortReason uint8
+
+// These are the various reasons why a test might have been aborted prematurely.
+const (
+	AbortedByUser AbortReason = iota + 1
+	AbortedByThreshold
+	AbortedByThresholdsAfterTestEnd // TODO: rename?
+	AbortedByScriptError
+	AbortedByScriptAbort
+	AbortedByTimeout
+)
+
+// HasAbortReason is a wrapper around an error with an attached abort reason.
+type HasAbortReason interface {
+	error
+	AbortReason() AbortReason
+}
+
+// WithAbortReasonIfNone can attach an abort reason to the given error, if it
+// doesn't have one already. It won't do anything if the error already had an
+// abort reason attached. Similarly, if there is no error (i.e. the given error
+// is nil), it also won't do anything and will return nil.
+func WithAbortReasonIfNone(err error, abortReason AbortReason) error {
+	if err == nil {
+		return nil // No error, do nothing
+	}
+	var arerr HasAbortReason
+	if errors.As(err, &arerr) {
+		// The given error already has an abort reason, do nothing
+		return err
+	}
+	return withAbortReason{err, abortReason}
+}
+
+type withAbortReason struct {
+	error
+	abortReason AbortReason
+}
+
+func (ar withAbortReason) Unwrap() error {
+	return ar.error
+}
+
+func (ar withAbortReason) AbortReason() AbortReason {
+	return ar.abortReason
+}
+
+var _ HasAbortReason = withAbortReason{}

errext/exception.go

@@ -5,5 +5,6 @@ package errext
 // a stack trace that lead to them.
 type Exception interface {
 	error
+	HasAbortReason
 	StackTrace() string
 }

errext/interrupt_error.go

@@ -11,7 +11,10 @@ type InterruptError struct {
 	Reason string
 }
 
-var _ HasExitCode = &InterruptError{}
+var _ interface {
+	HasExitCode
+	HasAbortReason
+} = &InterruptError{}
 
 // Error returns the reason of the interruption.
 func (i *InterruptError) Error() string {
@@ -23,6 +26,12 @@ func (i *InterruptError) ExitCode() exitcodes.ExitCode {
 	return exitcodes.ScriptAborted
 }
 
+// AbortReason is used to signal that an InterruptError is caused by the
+// test.abort() functin in k6/execution.
+func (i *InterruptError) AbortReason() AbortReason {
+	return AbortedByScriptAbort
+}
+
 // AbortTest is the reason emitted when a test script calls test.abort()
 const AbortTest = "test aborted"
 

js/runner.go

@@ -845,11 +845,12 @@ type scriptException struct {
 	inner *goja.Exception
 }
 
-var (
-	_ errext.Exception   = &scriptException{}
-	_ errext.HasExitCode = &scriptException{}
-	_ errext.HasHint     = &scriptException{}
-)
+var _ interface {
+	errext.Exception
+	errext.HasExitCode
+	errext.HasHint
+	errext.HasAbortReason
+} = &scriptException{}
 
 func (s *scriptException) Error() string {
 	// this calls String instead of error so that by default if it's printed to print the stacktrace
@@ -868,6 +869,10 @@ func (s *scriptException) Hint() string {
 	return "script exception"
 }
 
+func (s *scriptException) AbortReason() errext.AbortReason {
+	return errext.AbortedByScriptError
+}
+
 func (s *scriptException) ExitCode() exitcodes.ExitCode {
 	return exitcodes.ScriptException
 }

js/runner_test.go

@@ -391,7 +391,7 @@ func TestDataIsolation(t *testing.T) {
 	engine, err := core.NewEngine(testRunState, execScheduler, []output.Output{mockOutput})
 	require.NoError(t, err)
 	require.NoError(t, engine.OutputManager.StartOutputs())
-	defer engine.OutputManager.StopOutputs()
+	defer engine.OutputManager.StopOutputs(nil)
 
 	ctx, cancel := context.WithCancel(context.Background())
 	run, wait, err := engine.Init(ctx, ctx)

js/timeout_error.go

@@ -15,10 +15,11 @@ type timeoutError struct {
 	d     time.Duration
 }
 
-var (
-	_ errext.HasExitCode = timeoutError{}
-	_ errext.HasHint     = timeoutError{}
-)
+var _ interface {
+	errext.HasExitCode
+	errext.HasHint
+	errext.HasAbortReason
+} = timeoutError{}
 
 // newTimeoutError returns a new timeout error, reporting that a timeout has
 // happened at the given place and given duration.
@@ -44,6 +45,10 @@ func (t timeoutError) Hint() string {
 	return hint
 }
 
+func (t timeoutError) AbortReason() errext.AbortReason {
+	return errext.AbortedByTimeout
+}
+
 // ExitCode returns the coresponding exit code value to the place.
 func (t timeoutError) ExitCode() exitcodes.ExitCode {
 	// TODO: add handleSummary()