diff --git a/pql/ast.go b/pql/ast.go index 36d6e35fb..20b757946 100644 --- a/pql/ast.go +++ b/pql/ast.go @@ -119,7 +119,7 @@ func (q *Query) addField(field string) { // key/value. func (q *Query) validateArgField(elem *callStackElem) { if _, exists := elem.call.Args[elem.lastField]; exists { - panic(fmt.Sprintf("multiple instances of argument '%s' provided", elem.lastField)) + panic(fmt.Sprintf("%s: %s", duplicateArgErrorMessage, elem.lastField)) } } diff --git a/pql/parser.go b/pql/parser.go index 611294971..6a28f560e 100644 --- a/pql/parser.go +++ b/pql/parser.go @@ -15,6 +15,7 @@ package pql import ( + "fmt" "io" "io/ioutil" "strings" @@ -25,6 +26,9 @@ import ( // timeFormat is the go-style time format used to parse string dates. const timeFormat = "2006-01-02T15:04" +// duplicateArgErrorMessage is used as an error string in the parser. +const duplicateArgErrorMessage = "duplicate argument provided" + // parser represents a parser for the PQL language. type parser struct { r io.Reader @@ -59,6 +63,20 @@ func (p *parser) Parse() (*Query, error) { if err != nil { return nil, errors.Wrap(err, "parsing") } - p.Execute() + + // Handle specific panics from the parser and return them as errors. + var v interface{} + func() { + defer func() { v = recover() }() + p.Execute() + }() + if v != nil { + if strings.HasPrefix(v.(string), duplicateArgErrorMessage) { + return nil, fmt.Errorf("%s", v) + } else { + panic(v) + } + } + return &p.Query, nil } diff --git a/pql/pqlpeg_test.go b/pql/pqlpeg_test.go index f8a6597b5..a90d58a5d 100644 --- a/pql/pqlpeg_test.go +++ b/pql/pqlpeg_test.go @@ -1,6 +1,21 @@ +// Copyright 2017 Pilosa Corp. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package pql import ( + "fmt" "reflect" "strconv" "testing" @@ -677,7 +692,7 @@ func TestPQLDeepEquality(t *testing.T) { } } -func TestPQLPanic(t *testing.T) { +func TestDuplicateArgError(t *testing.T) { tests := []struct { name string call string @@ -710,21 +725,13 @@ func TestPQLPanic(t *testing.T) { } for i, test := range tests { t.Run(test.name+strconv.Itoa(i), func(t *testing.T) { - var v interface{} - func() { - defer func() { v = recover() }() - - q, err := ParseString(test.call) - if err != nil { - t.Fatalf("parsing query '%s': %v", test.call, err) - } - _ = q - }() - - if !reflect.DeepEqual(v, "multiple instances of argument 'a' provided") { - t.Fatalf("unexpected panic value: %#v", v) + _, err := ParseString(test.call) + expErr := fmt.Sprintf("%s: a", duplicateArgErrorMessage) + if err == nil { + t.Fatalf("expected error for duplicate argument: %s", test.call) + } else if err.Error() != expErr { + t.Fatalf("expected error: %s, but got: %v", expErr, err.Error()) } - }) } }