k6runner: inspect errors and propagate unexpected ones to the probe

internal/k6runner/k6runner.go

@@ -75,6 +75,11 @@ func NewScript(script []byte, k6runner Runner) (*Script, error) {
 	return &r, nil
 }
 
+var (
+	ErrBuggyRunner = errors.New("runner returned buggy response")
+	ErrFromRunner  = errors.New("runner reported an error")
+)
+
 func (r Script) Run(ctx context.Context, registry *prometheus.Registry, logger logger.Logger, internalLogger zerolog.Logger) (bool, error) {
 	k6runner := r.runner.WithLogger(&internalLogger)
 
@@ -112,8 +117,29 @@ func (r Script) Run(ctx context.Context, registry *prometheus.Registry, logger l
 		return false, err
 	}
 
-	success := result.Error == "" && result.ErrorCode == ""
-	return success, nil
+	// If only one of Error and ErrorCode are non-empty, the proxy is misbehaving.
+	switch {
+	case result.Error == "" && result.ErrorCode != "":
+		fallthrough
+	case result.Error != "" && result.ErrorCode == "":
+		return false, fmt.Errorf(
+			"%w: only one of error (%q) and errorCode (%q) is non-empty",
+			ErrBuggyRunner, result.Error, result.ErrorCode,
+		)
+	}
+
+	// https://github.com/grafana/sm-k6-runner/blob/b811839d444a7e69fd056b0a4e6ccf7e914197f3/internal/mq/runner.go#L51
+	switch result.ErrorCode {
+	case "":
+		// No error, all good.
+		return true, nil
+	case "timeout", "killed", "user":
+		// These are user errors. The probe failed, but we don't return an error.
+		return false, nil
+	default:
+		// We got an "unknown" error, or some other code we do not recognize. Return it so we log it.
+		return false, fmt.Errorf("%w: %s: %s", ErrFromRunner, result.ErrorCode, result.Error)
+	}
 }
 
 type customCollector struct {

internal/k6runner/k6runner_test.go

@@ -192,40 +192,63 @@ func TestScriptHTTPRun(t *testing.T) {
 			expectError:   nil,
 		},
 		{
-			// HTTP runner returns an error when the upstream status is not recognized.
-			// Script should report that error and failure.
+			// HTTP runner should report failure and an error when the upstream status is not recognized.
 			name:          "unexpected status",
 			response:      &RunResponse{},
 			statusCode:    999,
 			expectSuccess: false,
 			expectError:   ErrUnexpectedStatus,
 		},
 		{
-			// HTTP runner should report failure but no error if there is an error in the response.
-			// Other than checking for known status codes, it is ignored in this logic.
-			name: "error in response status 200",
+			// HTTP runner should report failure and an error when the error is unknown.
+			name: "non-user error",
 			response: &RunResponse{
 				Metrics:   testMetrics,
 				Logs:      testLogs,
+				Error:     "something went wrong",
 				ErrorCode: "something-wrong",
 			},
 			statusCode:    http.StatusOK,
 			expectSuccess: false,
-			expectError:   nil,
+			expectError:   ErrFromRunner,
 		},
 		{
-			// HTTP runner should report failure but no error if there is an error in the response.
-			// Other than checking for known status codes, it is ignored in this logic.
-			name: "error in response status 4XX",
+			// HTTP runner should report failure but no error when the error is unknown.
+			name: "user error",
 			response: &RunResponse{
 				Metrics:   testMetrics,
 				Logs:      testLogs,
-				ErrorCode: "something-wrong",
+				Error:     "syntax error somewhere or something",
+				ErrorCode: "user",
 			},
-			statusCode:    http.StatusUnprocessableEntity,
+			statusCode:    http.StatusOK,
 			expectSuccess: false,
 			expectError:   nil,
 		},
+		{
+			name: "inconsistent runner response A",
+			response: &RunResponse{
+				Metrics:   testMetrics,
+				Logs:      testLogs,
+				Error:     "set",
+				ErrorCode: "",
+			},
+			statusCode:    http.StatusInternalServerError,
+			expectSuccess: false,
+			expectError:   ErrBuggyRunner,
+		},
+		{
+			name: "inconsistent runner response B",
+			response: &RunResponse{
+				Metrics:   testMetrics,
+				Logs:      testLogs,
+				Error:     "",
+				ErrorCode: "set",
+			},
+			statusCode:    http.StatusInternalServerError,
+			expectSuccess: false,
+			expectError:   ErrBuggyRunner,
+		},
 	} {
 		tc := tc