js: Print stack trace when exception in handleSummary() (#3416)

Co-authored-by: Mihail Stoykov <312246+mstoykov@users.noreply.github.com>

cmd/root.go

@@ -112,18 +112,7 @@ func (c *rootCommand) execute() {
 		exitCode = int(ecerr.ExitCode())
 	}
 
-	errText := err.Error()
-	var xerr errext.Exception
-	if errors.As(err, &xerr) {
-		errText = xerr.StackTrace()
-	}
-
-	fields := logrus.Fields{}
-	var herr errext.HasHint
-	if errors.As(err, &herr) {
-		fields["hint"] = herr.Hint()
-	}
-
+	errText, fields := errext.Format(err)
 	c.globalState.Logger.WithFields(fields).Error(errText)
 	if c.loggerIsRemote {
 		c.globalState.FallbackLogger.WithFields(fields).Error(errText)

errext/format.go

@@ -0,0 +1,28 @@
+package errext
+
+import (
+	"errors"
+)
+
+// Format formats the given error as a message (string) and a map of fields.
+// In case of [Exception], it uses the stack trace as the error message.
+// In case of [HasHint], it also adds the hint as a field.
+func Format(err error) (string, map[string]interface{}) {
+	if err == nil {
+		return "", nil
+	}
+
+	errText := err.Error()
+	var xerr Exception
+	if errors.As(err, &xerr) {
+		errText = xerr.StackTrace()
+	}
+
+	fields := make(map[string]interface{})
+	var herr HasHint
+	if errors.As(err, &herr) {
+		fields["hint"] = herr.Hint()
+	}
+
+	return errText, fields
+}

errext/format_test.go

@@ -0,0 +1,70 @@
+package errext_test
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"go.k6.io/k6/errext"
+)
+
+func TestFormat(t *testing.T) {
+	t.Parallel()
+
+	t.Run("Nil", func(t *testing.T) {
+		t.Parallel()
+		errorText, fields := errext.Format(nil)
+		assert.Equal(t, "", errorText)
+		assert.Empty(t, fields)
+	})
+
+	t.Run("Simple", func(t *testing.T) {
+		t.Parallel()
+		errorText, fields := errext.Format(errors.New("simple error"))
+		assert.Equal(t, "simple error", errorText)
+		assert.Empty(t, fields)
+	})
+
+	t.Run("Exception", func(t *testing.T) {
+		t.Parallel()
+		err := fakeException{error: errors.New("simple error"), stack: "stack trace"}
+		errorText, fields := errext.Format(err)
+		assert.Equal(t, "stack trace", errorText)
+		assert.Empty(t, fields)
+	})
+
+	t.Run("Hint", func(t *testing.T) {
+		t.Parallel()
+		err := errext.WithHint(errors.New("error with hint"), "hint message")
+		errorText, fields := errext.Format(err)
+		assert.Equal(t, "error with hint", errorText)
+		assert.Equal(t, map[string]interface{}{"hint": "hint message"}, fields)
+	})
+
+	t.Run("ExceptionWithHint", func(t *testing.T) {
+		t.Parallel()
+		err := fakeException{error: errext.WithHint(errors.New("error with hint"), "hint message"), stack: "stack trace"}
+		errorText, fields := errext.Format(err)
+		assert.Equal(t, "stack trace", errorText)
+		assert.Equal(t, map[string]interface{}{"hint": "hint message"}, fields)
+	})
+}
+
+type fakeException struct {
+	error
+	stack string
+	abort errext.AbortReason
+}
+
+func (e fakeException) StackTrace() string {
+	return e.stack
+}
+
+func (e fakeException) AbortReason() errext.AbortReason {
+	return e.abort
+}
+
+func (e fakeException) Unwrap() error {
+	return e.error
+}

js/runner.go

@@ -388,20 +388,27 @@ func (r *Runner) HandleSummary(ctx context.Context, summary *lib.Summary) (map[s
 		return nil, err
 	}
 
-	handleSummaryFn := goja.Undefined()
-	fn := vu.getExported(consts.HandleSummaryFn)
-	if _, ok := goja.AssertFunction(fn); ok {
-		handleSummaryFn = fn
-	} else if fn != nil {
-		return nil, fmt.Errorf("exported identifier %s must be a function", consts.HandleSummaryFn)
-	}
-
 	go func() {
 		<-summaryCtx.Done()
 		vu.Runtime.Interrupt(context.Canceled)
 	}()
 	vu.moduleVUImpl.ctx = summaryCtx
 
+	callbackResult := goja.Undefined()
+	fn := vu.getExported(consts.HandleSummaryFn)
+	if fn != nil {
+		handleSummaryFn, ok := goja.AssertFunction(fn)
+		if !ok {
+			return nil, fmt.Errorf("exported identifier %s must be a function", consts.HandleSummaryFn)
+		}
+
+		callbackResult, _, _, err = vu.runFn(summaryCtx, false, handleSummaryFn, nil, vu.Runtime.ToValue(summaryDataForJS))
+		if err != nil {
+			errText, fields := errext.Format(err)
+			r.preInitState.Logger.WithFields(fields).Error(errText)
+		}
+	}
+
 	wrapper := strings.Replace(summaryWrapperLambdaCode, "/*JSLIB_SUMMARY_CODE*/", jslibSummaryCode, 1)
 	handleSummaryWrapperRaw, err := vu.Runtime.RunString(wrapper)
 	if err != nil {
@@ -413,7 +420,7 @@ func (r *Runner) HandleSummary(ctx context.Context, summary *lib.Summary) (map[s
 	}
 
 	wrapperArgs := []goja.Value{
-		handleSummaryFn,
+		callbackResult,
 		vu.Runtime.ToValue(r.Bundle.preInitState.RuntimeOptions.SummaryExport.String),
 		vu.Runtime.ToValue(summaryDataForJS),
 	}

js/summary-wrapper.js

@@ -59,24 +59,13 @@
         return JSON.stringify(results, null, 4);
     };
 
-    return function (exportedSummaryCallback, jsonSummaryPath, data) {
-        var getDefaultSummary = function () {
+    return function (summaryCallbackResult, jsonSummaryPath, data) {
+        var result = summaryCallbackResult;
+        if (!result) {
             var enableColors = (!data.options.noColor && data.state.isStdOutTTY);
-            return {
-                'stdout': '\n' + jslib.textSummary(data, { indent: ' ', enableColors: enableColors }) + '\n\n',
+            result = {
+                'stdout': '\n' + jslib.textSummary(data, {indent: ' ', enableColors: enableColors}) + '\n\n',
             };
-        };
-
-        var result = {};
-        if (exportedSummaryCallback) {
-            try {
-                result = exportedSummaryCallback(data);
-            } catch (e) {
-                console.error('handleSummary() failed with error "' + e + '", falling back to the default summary');
-                result = getDefaultSummary();
-            }
-        } else {
-            result = getDefaultSummary();
         }
 
         // TODO: ensure we're returning a map of strings or null/undefined...

js/summary_test.go

@@ -698,5 +698,6 @@ func TestExceptionInHandleSummaryFallsBackToTextSummary(t *testing.T) {
 	assert.Equal(t, 1, len(logErrors))
 	errMsg, err := logErrors[0].String()
 	require.NoError(t, err)
-	assert.Contains(t, errMsg, "intentional error")
+	assert.Contains(t, errMsg, "\"Error: intentional error\\n\\tat file:///script.js:5:11(3)\\n")
+	assert.Equal(t, logErrors[0].Data, logrus.Fields{"hint": "script exception"})
 }