Print query that caused a panic (#10995)

We print the stacktrace of a panic when query causes one, but there's no
information about the query itself, which makes it harder to debug and
reproduce the issue.
This adds the 'expr' string to the logged panic.

Signed-off-by: Łukasz Mierzwa <l.mierzwa@gmail.com>
This commit is contained in:
Łukasz Mierzwa 2022-07-14 10:34:15 +01:00 committed by GitHub
parent 409a5e7922
commit 54a3c3ba3f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 19 additions and 9 deletions

View file

@ -937,7 +937,7 @@ func (ev *evaluator) error(err error) {
} }
// recover is the handler that turns panics into returns from the top level of evaluation. // recover is the handler that turns panics into returns from the top level of evaluation.
func (ev *evaluator) recover(ws *storage.Warnings, errp *error) { func (ev *evaluator) recover(expr parser.Expr, ws *storage.Warnings, errp *error) {
e := recover() e := recover()
if e == nil { if e == nil {
return return
@ -949,7 +949,7 @@ func (ev *evaluator) recover(ws *storage.Warnings, errp *error) {
buf := make([]byte, 64<<10) buf := make([]byte, 64<<10)
buf = buf[:runtime.Stack(buf, false)] buf = buf[:runtime.Stack(buf, false)]
level.Error(ev.logger).Log("msg", "runtime panic in parser", "err", e, "stacktrace", string(buf)) level.Error(ev.logger).Log("msg", "runtime panic in parser", "expr", expr.String(), "err", e, "stacktrace", string(buf))
*errp = fmt.Errorf("unexpected error: %w", err) *errp = fmt.Errorf("unexpected error: %w", err)
case errWithWarnings: case errWithWarnings:
*errp = err.err *errp = err.err
@ -960,7 +960,7 @@ func (ev *evaluator) recover(ws *storage.Warnings, errp *error) {
} }
func (ev *evaluator) Eval(expr parser.Expr) (v parser.Value, ws storage.Warnings, err error) { func (ev *evaluator) Eval(expr parser.Expr) (v parser.Value, ws storage.Warnings, err error) {
defer ev.recover(&ws, &err) defer ev.recover(expr, &ws, &err)
v, ws = ev.eval(expr) v, ws = ev.eval(expr)
return v, ws, nil return v, ws, nil

View file

@ -1641,17 +1641,27 @@ load 1ms
} }
func TestRecoverEvaluatorRuntime(t *testing.T) { func TestRecoverEvaluatorRuntime(t *testing.T) {
ev := &evaluator{logger: log.NewNopLogger()} var output []interface{}
logger := log.Logger(log.LoggerFunc(func(keyvals ...interface{}) error {
output = append(output, keyvals...)
return nil
}))
ev := &evaluator{logger: logger}
expr, _ := parser.ParseExpr("sum(up)")
var err error var err error
defer ev.recover(nil, &err)
defer func() {
require.EqualError(t, err, "unexpected error: runtime error: index out of range [123] with length 0")
require.Contains(t, output, "sum(up)")
}()
defer ev.recover(expr, nil, &err)
// Cause a runtime panic. // Cause a runtime panic.
var a []int var a []int
//nolint:govet //nolint:govet
a[123] = 1 a[123] = 1
require.EqualError(t, err, "unexpected error")
} }
func TestRecoverEvaluatorError(t *testing.T) { func TestRecoverEvaluatorError(t *testing.T) {
@ -1663,7 +1673,7 @@ func TestRecoverEvaluatorError(t *testing.T) {
defer func() { defer func() {
require.EqualError(t, err, e.Error()) require.EqualError(t, err, e.Error())
}() }()
defer ev.recover(nil, &err) defer ev.recover(nil, nil, &err)
panic(e) panic(e)
} }
@ -1683,7 +1693,7 @@ func TestRecoverEvaluatorErrorWithWarnings(t *testing.T) {
require.EqualError(t, err, e.Error()) require.EqualError(t, err, e.Error())
require.Equal(t, warnings, ws, "wrong warning message") require.Equal(t, warnings, ws, "wrong warning message")
}() }()
defer ev.recover(&ws, &err) defer ev.recover(nil, &ws, &err)
panic(e) panic(e)
} }