Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Nov 1, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 1 addition & 12 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
28 changes: 28 additions & 0 deletions errext/format.go
Original file line number Diff line number Diff line change
@@ -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.
joanlopez marked this conversation as resolved.
Show resolved Hide resolved
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
}
70 changes: 70 additions & 0 deletions errext/format_test.go
Original file line number Diff line number Diff line change
@@ -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
}
25 changes: 16 additions & 9 deletions js/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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),
}
Expand Down
21 changes: 5 additions & 16 deletions js/summary-wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -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...
Expand Down
3 changes: 2 additions & 1 deletion js/summary_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"})
}