Skip to content

Commit

Permalink
Fix panic handling and add a new exit code and a test for panics (#2997)
Browse files Browse the repository at this point in the history
Because we started calling os.Exit() in a defer statement, we circumvented the regular handling of panics that bubble up to the top by the Go runtime. I could have removed the os.Exit() call from the defer, but handling panics ourselves in this way allows us to handle them exactly how we want, assign them their own unique exit code, and test the whole thing with an e2e test.
  • Loading branch information
na-- authored Mar 31, 2023
1 parent 32eb7e3 commit c09919b
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 0 deletions.
13 changes: 13 additions & 0 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io"
stdlog "log"
"runtime/debug"
"strconv"
"strings"
"sync"
Expand All @@ -17,6 +18,7 @@ import (

"go.k6.io/k6/cmd/state"
"go.k6.io/k6/errext"
"go.k6.io/k6/errext/exitcodes"
"go.k6.io/k6/lib/consts"
"go.k6.io/k6/log"
)
Expand Down Expand Up @@ -88,6 +90,17 @@ func (c *rootCommand) execute() {
c.globalState.OSExit(exitCode)
}()

defer func() {
if r := recover(); r != nil {
exitCode = int(exitcodes.GoPanic)
err := fmt.Errorf("unexpected k6 panic: %s\n%s", r, debug.Stack())
if c.loggerIsRemote {
c.globalState.FallbackLogger.Error(err)
}
c.globalState.Logger.Error(err)
}
}()

err := c.cmd.Execute()
if err == nil {
exitCode = 0
Expand Down
24 changes: 24 additions & 0 deletions cmd/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ 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"
)

Expand Down Expand Up @@ -37,3 +39,25 @@ func TestDeprecatedOptionWarning(t *testing.T) {
assert.False(t, testutils.LogContains(logMsgs, logrus.InfoLevel, "logformat"))
assert.Contains(t, ts.Stdout.String(), `--logformat has been deprecated`)
}

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(cmd *cobra.Command, args []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
}
1 change: 1 addition & 0 deletions errext/exitcodes/codes.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ const (
CannotStartRESTAPI ExitCode = 106
ScriptException ExitCode = 107
ScriptAborted ExitCode = 108
GoPanic ExitCode = 109
)

0 comments on commit c09919b

Please sign in to comment.