diff --git a/cmd/panic_integration_test.go b/cmd/panic_integration_test.go deleted file mode 100644 index 40fc9468276..00000000000 --- a/cmd/panic_integration_test.go +++ /dev/null @@ -1,101 +0,0 @@ -package cmd - -// TODO: convert this into the integration tests, once https://github.com/grafana/k6/issues/2459 will be done - -import ( - "path/filepath" - "testing" - - "github.com/sirupsen/logrus" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "go.k6.io/k6/cmd/tests" - "go.k6.io/k6/errext/exitcodes" - "go.k6.io/k6/js/modules" - "go.k6.io/k6/lib/fsext" - "go.k6.io/k6/lib/testutils" -) - -// alarmist is a mock module that do a panic -type alarmist struct { - vu modules.VU -} - -var _ modules.Module = &alarmist{} - -func (a *alarmist) NewModuleInstance(vu modules.VU) modules.Instance { - return &alarmist{ - vu: vu, - } -} - -func (a *alarmist) Exports() modules.Exports { - return modules.Exports{ - Named: map[string]interface{}{ - "panic": a.panic, - }, - } -} - -func (a *alarmist) panic(s string) { - panic(s) -} - -func init() { - modules.Register("k6/x/alarmist", new(alarmist)) -} - -func TestRunScriptPanicsErrorsAndAbort(t *testing.T) { - t.Parallel() - - testCases := []struct { - caseName, testScript, expectedLogMessage string - }{ - { - caseName: "panic in the VU context", - testScript: ` - import { panic } from 'k6/x/alarmist'; - - export default function() { - panic('hey'); - console.log('lorem ipsum'); - } - `, - expectedLogMessage: "a panic occurred during JS execution: hey", - }, - { - caseName: "panic in the init context", - testScript: ` - import { panic } from 'k6/x/alarmist'; - - panic('hey'); - export default function() { - console.log('lorem ipsum'); - } - `, - expectedLogMessage: "a panic occurred during JS execution: hey", - }, - } - - for _, tc := range testCases { - tc := tc - name := tc.caseName - - t.Run(name, func(t *testing.T) { - t.Parallel() - - testFilename := "script.js" - ts := tests.NewGlobalTestState(t) - require.NoError(t, fsext.WriteFile(ts.FS, filepath.Join(ts.Cwd, testFilename), []byte(tc.testScript), 0o644)) - ts.CmdArgs = []string{"k6", "run", testFilename} - - ts.ExpectedExitCode = int(exitcodes.ScriptAborted) - newRootCommand(ts.GlobalState).execute() - - logs := ts.LoggerHook.Drain() - - assert.True(t, testutils.LogContains(logs, logrus.ErrorLevel, tc.expectedLogMessage)) - assert.False(t, testutils.LogContains(logs, logrus.InfoLevel, "lorem ipsum")) - }) - } -} diff --git a/cmd/root_test.go b/cmd/root_test.go deleted file mode 100644 index be8deff5cb4..00000000000 --- a/cmd/root_test.go +++ /dev/null @@ -1,38 +0,0 @@ -package cmd - -import ( - "testing" - - "github.com/sirupsen/logrus" - "github.com/spf13/cobra" - "github.com/stretchr/testify/assert" - "go.k6.io/k6/cmd/tests" - "go.k6.io/k6/errext/exitcodes" - "go.k6.io/k6/lib/testutils" -) - -func TestMain(m *testing.M) { - tests.Main(m) -} - -func TestPanicHandling(t *testing.T) { - t.Parallel() - - ts := tests.NewGlobalTestState(t) - ts.CmdArgs = []string{"k6", "panic"} - ts.ExpectedExitCode = int(exitcodes.GoPanic) - - rootCmd := newRootCommand(ts.GlobalState) - rootCmd.cmd.AddCommand(&cobra.Command{ - Use: "panic", - RunE: func(_ *cobra.Command, _ []string) error { - panic("oh no, oh no, oh no,no,no,no,no") - }, - }) - rootCmd.execute() - - t.Log(ts.Stderr.String()) - logMsgs := ts.LoggerHook.Drain() - assert.True(t, testutils.LogContains(logMsgs, logrus.ErrorLevel, "unexpected k6 panic: oh no")) - assert.True(t, testutils.LogContains(logMsgs, logrus.ErrorLevel, "cmd.TestPanicHandling")) // check stacktrace -} diff --git a/js/bundle.go b/js/bundle.go index b6cd546cd04..3f2db060253 100644 --- a/js/bundle.go +++ b/js/bundle.go @@ -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 { @@ -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 diff --git a/js/common/util.go b/js/common/util.go index 868527a9416..94e8cff0387 100644 --- a/js/common/util.go +++ b/js/common/util.go @@ -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. @@ -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) diff --git a/js/runner.go b/js/runner.go index 3c69fe74381..64c116094b7 100644 --- a/js/runner.go +++ b/js/runner.go @@ -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 { diff --git a/js/runner_test.go b/js/runner_test.go index 305dada40b5..f755d20b317 100644 --- a/js/runner_test.go +++ b/js/runner_test.go @@ -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