Skip to content

Commit

Permalink
Stop catching panics
Browse files Browse the repository at this point in the history
  • Loading branch information
olegbespalov committed Jun 6, 2024
1 parent e93c816 commit 75615bc
Show file tree
Hide file tree
Showing 6 changed files with 8 additions and 243 deletions.
101 changes: 0 additions & 101 deletions cmd/panic_integration_test.go

This file was deleted.

38 changes: 0 additions & 38 deletions cmd/root_test.go

This file was deleted.

12 changes: 5 additions & 7 deletions js/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ func (b *Bundle) instantiate(vuImpl *moduleVUImpl, vuID uint64) (*sobek.Object,
}()

// TODO: make something cleaner for interrupting scripts, and more unified
// (e.g. as a part of the event loop or RunWithPanicCatching()?
// (e.g. as a part of the event loop?
initDone := make(chan struct{})
go func() {
select {
Expand All @@ -302,12 +302,10 @@ func (b *Bundle) instantiate(vuImpl *moduleVUImpl, vuID uint64) (*sobek.Object,
}()

var exportsV sobek.Value
err = common.RunWithPanicCatching(b.preInitState.Logger, rt, func() error {
return vuImpl.eventLoop.Start(func() error {
var err error
exportsV, err = modSys.RunSourceData(b.sourceData)
return err
})
err = vuImpl.eventLoop.Start(func() error {
var err error
exportsV, err = modSys.RunSourceData(b.sourceData)
return err
})

<-initDone
Expand Down
19 changes: 0 additions & 19 deletions js/common/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,8 @@ import (
"bytes"
"fmt"
"io"
"runtime/debug"

"github.com/grafana/sobek"
"github.com/sirupsen/logrus"
"go.k6.io/k6/errext"
)

// Throw a JS error; avoids re-wrapping GoErrors.
Expand Down Expand Up @@ -64,22 +61,6 @@ func ToString(data interface{}) (string, error) {
}
}

// RunWithPanicCatching catches panic and converts into an InterruptError error that should abort a script
func RunWithPanicCatching(logger logrus.FieldLogger, _ *sobek.Runtime, fn func() error) (err error) {
defer func() {
if r := recover(); r != nil {
err = &errext.InterruptError{Reason: fmt.Sprintf("a panic occurred during JS execution: %s", r)}
// TODO figure out how to use PanicLevel without panicing .. this might require changing
// the logger we use see
// https://github.com/sirupsen/logrus/issues/1028
// https://github.com/sirupsen/logrus/issues/993
logger.Error("panic: ", r, "\n", string(debug.Stack()))
}
}()

return fn()
}

// IsNullish checks if the given value is nullish, i.e. nil, undefined or null.
func IsNullish(v sobek.Value) bool {
return v == nil || sobek.IsUndefined(v) || sobek.IsNull(v)
Expand Down
8 changes: 3 additions & 5 deletions js/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -840,11 +840,9 @@ func (u *VU) runFn(
if u.moduleVUImpl.eventLoop == nil {
u.moduleVUImpl.eventLoop = eventloop.New(u.moduleVUImpl)
}
err = common.RunWithPanicCatching(u.state.Logger, u.Runtime, func() error {
return u.moduleVUImpl.eventLoop.Start(func() (err error) {
v, err = fn(sobek.Undefined(), args...) // Actually run the JS script
return err
})
err = u.moduleVUImpl.eventLoop.Start(func() (err error) {
v, err = fn(sobek.Undefined(), args...) // Actually run the JS script
return err
})

select {
Expand Down
73 changes: 0 additions & 73 deletions js/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2230,79 +2230,6 @@ func TestSystemTags(t *testing.T) {
}
}

func TestVUPanic(t *testing.T) {
t.Parallel()
r1, err := getSimpleRunner(t, "/script.js", `
var group = require("k6").group;
exports.default = function() {
group("panic here", function() {
if (__ITER == 0) {
panic("here we panic");
}
console.log("here we don't");
})
}`,
)
require.NoError(t, err)

registry := metrics.NewRegistry()
builtinMetrics := metrics.RegisterBuiltinMetrics(registry)
r2, err := NewFromArchive(
&lib.TestPreInitState{
Logger: testutils.NewLogger(t),
BuiltinMetrics: builtinMetrics,
Registry: registry,
}, r1.MakeArchive())
require.NoError(t, err)

runners := map[string]*Runner{"Source": r1, "Archive": r2}
for name, r := range runners {
r := r
t.Run(name, func(t *testing.T) {
t.Parallel()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

initVU, err := r.NewVU(ctx, 1, 1234, make(chan metrics.SampleContainer, 100))
require.NoError(t, err)

logger := logrus.New()
logger.SetLevel(logrus.InfoLevel)
logger.Out = io.Discard
hook := testutils.NewLogHook(
logrus.InfoLevel, logrus.ErrorLevel, logrus.FatalLevel, logrus.PanicLevel,
)
logger.AddHook(hook)

vu := initVU.Activate(&lib.VUActivationParams{RunContext: ctx})
activeVU, ok := vu.(*ActiveVU)
require.True(t, ok)
require.NoError(t, activeVU.Runtime.Set("panic", func(str string) { panic(str) }))
activeVU.state.Logger = logger

activeVU.Console.logger = logger.WithField("source", "console")
err = vu.RunOnce()
require.Error(t, err)
assert.Contains(t, err.Error(), "a panic occurred during JS execution: here we panic")
entries := hook.Drain()
require.Len(t, entries, 1)
assert.Equal(t, logrus.ErrorLevel, entries[0].Level)
require.True(t, strings.HasPrefix(entries[0].Message, "panic: here we panic"))
// broken since goja@f3cfc97811c0b4d8337902c3e42fb2371ba1d524 see
// https://github.com/dop251/goja/issues/179#issuecomment-783572020
// require.True(t, strings.HasSuffix(entries[0].Message, "Goja stack:\nfile:///script.js:3:4(12)"))

err = vu.RunOnce()
require.NoError(t, err)

entries = hook.Drain()
require.Len(t, entries, 1)
assert.Equal(t, logrus.InfoLevel, entries[0].Level)
require.Contains(t, entries[0].Message, "here we don't")
})
}
}

type multiFileTestCase struct {
fses map[string]fsext.Fs
rtOpts lib.RuntimeOptions
Expand Down

0 comments on commit 75615bc

Please sign in to comment.