Apply review feedback

cmd/root.go

@@ -112,9 +112,10 @@ func (c *rootCommand) execute() {
 		exitCode = int(ecerr.ExitCode())
 	}
 
-	errext.Fprint(c.globalState.Logger, err)
+	errText, fields := errext.Format(err)
+	c.globalState.Logger.WithFields(fields).Error(errText)
 	if c.loggerIsRemote {
-		errext.Fprint(c.globalState.FallbackLogger, err)
+		c.globalState.FallbackLogger.WithFields(fields).Error(errText)
 	}
 }
 

errext/print.go → errext/format.go

@@ -2,16 +2,14 @@ package errext
 
 import (
 	"errors"
-
-	"github.com/sirupsen/logrus"
 )
 
-// Fprint formats the given error and writes it to l.
+// Format formats the given error and writes it to l.
 // 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 Fprint(l logrus.FieldLogger, err error) {
+func Format(err error) (string, map[string]interface{}) {
 	if err == nil {
-		return
+		return "", nil
 	}
 
 	errText := err.Error()
@@ -20,11 +18,11 @@ func Fprint(l logrus.FieldLogger, err error) {
 		errText = xerr.StackTrace()
 	}
 
-	fields := logrus.Fields{}
+	fields := make(map[string]interface{})
 	var herr HasHint
 	if errors.As(err, &herr) {
 		fields["hint"] = herr.Hint()
 	}
 
-	l.WithFields(fields).Error(errText)
+	return errText, fields
 }

errext/print_test.go → errext/format_test.go

@@ -1,61 +1,53 @@
 package errext_test
 
 import (
-	"bytes"
 	"errors"
 	"testing"
 
-	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
+
 	"go.k6.io/k6/errext"
 )
 
-func TestFprint(t *testing.T) {
+func TestFormat(t *testing.T) {
 	t.Parallel()
 
-	init := func() (*bytes.Buffer, logrus.FieldLogger) {
-		var buf bytes.Buffer
-		logger := logrus.New()
-		logger.Out = &buf
-		return &buf, logger
-	}
-
 	t.Run("Nil", func(t *testing.T) {
 		t.Parallel()
-		buf, logger := init()
-		errext.Fprint(logger, nil)
-		assert.Equal(t, "", buf.String())
+		errorText, fields := errext.Format(nil)
+		assert.Equal(t, "", errorText)
+		assert.Empty(t, fields)
 	})
 
 	t.Run("Simple", func(t *testing.T) {
 		t.Parallel()
-		buf, logger := init()
-		errext.Fprint(logger, errors.New("simple error"))
-		assert.Contains(t, buf.String(), "level=error msg=\"simple error\"")
+		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()
-		buf, logger := init()
 		err := fakeException{error: errors.New("simple error"), stack: "stack trace"}
-		errext.Fprint(logger, err)
-		assert.Contains(t, buf.String(), "level=error msg=\"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()
-		buf, logger := init()
 		err := errext.WithHint(errors.New("error with hint"), "hint message")
-		errext.Fprint(logger, err)
-		assert.Contains(t, buf.String(), "level=error msg=\"error with hint\" 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()
-		buf, logger := init()
 		err := fakeException{error: errext.WithHint(errors.New("error with hint"), "hint message"), stack: "stack trace"}
-		errext.Fprint(logger, err)
-		assert.Contains(t, buf.String(), "level=error msg=\"stack trace\" hint=\"hint message\"")
+		errorText, fields := errext.Format(err)
+		assert.Equal(t, "stack trace", errorText)
+		assert.Equal(t, map[string]interface{}{"hint": "hint message"}, fields)
 	})
 }
 

js/runner.go

@@ -397,14 +397,16 @@ func (r *Runner) HandleSummary(ctx context.Context, summary *lib.Summary) (map[s
 	callbackResult := goja.Undefined()
 	fn := vu.getExported(consts.HandleSummaryFn)
 	if fn != nil {
-		if handleSummaryFn, ok := goja.AssertFunction(fn); ok {
-			callbackResult, _, _, err = vu.runFn(summaryCtx, false, handleSummaryFn, nil, vu.Runtime.ToValue(summaryDataForJS))
-			if err != nil {
-				errext.Fprint(r.preInitState.Logger, err)
-			}
-		} else 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)

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"})
 }