recover the duplicate arg panic from parser, treat as error

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))
 	}
 }
 

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
 }

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())
 			}
-
 		})
 	}
 }