Convert ConverterError to InvalidArgument in standard visibility pars…

…er (#2437)
temporalio · Feb 2, 2022 · 3eb7730 · 3eb7730
1 parent 9fc6b63
commit 3eb7730
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 16 deletions.
diff --git a/common/persistence/visibility/store/elasticsearch/visibility_store.go b/common/persistence/visibility/store/elasticsearch/visibility_store.go
@@ -39,6 +39,7 @@ import (
 	"github.com/olivere/elastic/v7"
 	enumspb "go.temporal.io/api/enums/v1"
 	"go.temporal.io/api/serviceerror"
+
 	"go.temporal.io/server/common/dynamicconfig"
 	"go.temporal.io/server/common/metrics"
 	"go.temporal.io/server/common/namespace"
@@ -639,10 +640,10 @@ func (s *visibilityStore) convertQuery(namespace namespace.Name, namespaceID nam
 	)
 	requestQuery, fieldSorts, err := queryConverter.ConvertWhereOrderBy(requestQueryStr)
 	if err != nil {
-		// Convert query.ConverterError to serviceerror.InvalidArgument and pass through all other errors (which should be only mapper errors).
+		// Convert ConverterError to InvalidArgument and pass through all other errors (which should be only mapper errors).
 		var converterErr *query.ConverterError
 		if errors.As(err, &converterErr) {
-			return nil, nil, serviceerror.NewInvalidArgument(fmt.Sprintf("unable to parse query: %v", converterErr))
+			return nil, nil, converterErr.ToInvalidArgument()
 		}
 		return nil, nil, err
 	}

diff --git a/common/persistence/visibility/store/elasticsearch/visibility_store_read_test.go b/common/persistence/visibility/store/elasticsearch/visibility_store_read_test.go
@@ -534,7 +534,7 @@ func (s *ESVisibilitySuite) Test_convertQuery() {
 	qry, srt, err = s.visibilityStore.convertQuery(testNamespace, testNamespaceID, query)
 	s.Error(err)
 	s.IsType(&serviceerror.InvalidArgument{}, err)
-	s.Equal(err.(*serviceerror.InvalidArgument).Error(), "unable to parse query: unable to convert 'order by' column name: unable to sort by field of Text type, use field of type Keyword")
+	s.Equal(err.(*serviceerror.InvalidArgument).Error(), "invalid query: unable to convert 'order by' column name: unable to sort by field of Text type, use field of type Keyword")
 
 	query = `order by CustomIntField asc`
 	qry, srt, err = s.visibilityStore.convertQuery(testNamespace, testNamespaceID, query)
@@ -591,7 +591,7 @@ func (s *ESVisibilitySuite) Test_convertQuery_Mapper() {
 	qry, srt, err = s.visibilityStore.convertQuery(testNamespace, testNamespaceID, query)
 	s.Error(err)
 	s.ErrorAs(err, &invalidArgumentErr)
-	s.EqualError(err, "unable to parse query: unable to convert filter expression: unable to convert left part of comparison expression: invalid search attribute: AliasForUnknownField")
+	s.EqualError(err, "invalid query: unable to convert filter expression: unable to convert left part of comparison expression: invalid search attribute: AliasForUnknownField")
 
 	query = `order by ExecutionTime`
 	qry, srt, err = s.visibilityStore.convertQuery(testNamespace, testNamespaceID, query)
@@ -615,7 +615,7 @@ func (s *ESVisibilitySuite) Test_convertQuery_Mapper() {
 	qry, srt, err = s.visibilityStore.convertQuery(testNamespace, testNamespaceID, query)
 	s.Error(err)
 	s.ErrorAs(err, &invalidArgumentErr)
-	s.EqualError(err, "unable to parse query: unable to convert 'order by' column name: invalid search attribute: AliasForUnknownField")
+	s.EqualError(err, "invalid query: unable to convert 'order by' column name: invalid search attribute: AliasForUnknownField")
 	s.visibilityStore.searchAttributesMapper = nil
 }
 
@@ -952,7 +952,7 @@ func (s *ESVisibilitySuite) TestListWorkflowExecutions() {
 	s.Error(err)
 	_, ok = err.(*serviceerror.InvalidArgument)
 	s.True(ok)
-	s.True(strings.HasPrefix(err.Error(), "unable to parse query"))
+	s.True(strings.HasPrefix(err.Error(), "invalid query"))
 }
 
 func (s *ESVisibilitySuite) TestListWorkflowExecutions_Error() {
@@ -1027,7 +1027,7 @@ func (s *ESVisibilitySuite) TestScanWorkflowExecutionsV6() {
 	s.Error(err)
 	_, ok := err.(*serviceerror.InvalidArgument)
 	s.True(ok)
-	s.True(strings.HasPrefix(err.Error(), "unable to parse query"))
+	s.True(strings.HasPrefix(err.Error(), "invalid query"))
 
 	// test scroll
 	scrollID := "scrollID-1"
@@ -1092,7 +1092,7 @@ func (s *ESVisibilitySuite) TestScanWorkflowExecutionsV7_Scroll() {
 	s.Error(err)
 	_, ok := err.(*serviceerror.InvalidArgument)
 	s.True(ok)
-	s.True(strings.HasPrefix(err.Error(), "unable to parse query"))
+	s.True(strings.HasPrefix(err.Error(), "invalid query"))
 
 	// test scroll
 	scrollID := "scrollID-1"
@@ -1167,7 +1167,7 @@ func (s *ESVisibilitySuite) TestScanWorkflowExecutionsV7_PIT() {
 	s.Error(err)
 	_, ok := err.(*serviceerror.InvalidArgument)
 	s.True(ok)
-	s.True(strings.HasPrefix(err.Error(), "unable to parse query"))
+	s.True(strings.HasPrefix(err.Error(), "invalid query"))
 
 	// test search
 	request.Query = `ExecutionStatus = "Terminated"`
@@ -1252,7 +1252,7 @@ func (s *ESVisibilitySuite) TestCountWorkflowExecutions() {
 	s.Error(err)
 	_, ok = err.(*serviceerror.InvalidArgument)
 	s.True(ok)
-	s.True(strings.HasPrefix(err.Error(), "unable to parse query"))
+	s.True(strings.HasPrefix(err.Error(), "invalid query"), err.Error())
 }
 
 func (s *ESVisibilitySuite) Test_detailedErrorMessage() {

diff --git a/common/persistence/visibility/store/query/errors.go b/common/persistence/visibility/store/query/errors.go
@@ -27,11 +27,13 @@ package query
 import (
 	"errors"
 	"fmt"
+
+	"go.temporal.io/api/serviceerror"
 )
 
 type (
 	ConverterError struct {
-		text string
+		message string
 	}
 )
 
@@ -42,12 +44,16 @@ var (
 )
 
 func NewConverterError(format string, a ...interface{}) error {
-	text := fmt.Sprintf(format, a...)
-	return &ConverterError{text: text}
+	message := fmt.Sprintf(format, a...)
+	return &ConverterError{message: message}
 }
 
 func (c *ConverterError) Error() string {
-	return c.text
+	return c.message
+}
+
+func (c *ConverterError) ToInvalidArgument() error {
+	return serviceerror.NewInvalidArgument(fmt.Sprintf("invalid query: %v", c))
 }
 
 func wrapConverterError(message string, err error) error {

diff --git a/common/persistence/visibility/store/standard/converter.go b/common/persistence/visibility/store/standard/converter.go
@@ -25,11 +25,12 @@
 package standard
 
 import (
-	"fmt"
+	"errors"
 	"time"
 
 	"github.com/xwb1989/sqlparser"
 	enumspb "go.temporal.io/api/enums/v1"
+
 	"go.temporal.io/server/common/persistence/sql/sqlplugin"
 	"go.temporal.io/server/common/persistence/visibility/store/query"
 )
@@ -69,7 +70,12 @@ func newQueryConverter() *converter {
 func (c *converter) GetFilter(whereOrderBy string) (*sqlplugin.VisibilitySelectFilter, error) {
 	_, _, err := c.ConvertWhereOrderBy(whereOrderBy)
 	if err != nil {
-		return nil, fmt.Errorf("invalid query: %v", err)
+		// Convert ConverterError to InvalidArgument and pass through all other errors.
+		var converterErr *query.ConverterError
+		if errors.As(err, &converterErr) {
+			return nil, converterErr.ToInvalidArgument()
+		}
+		return nil, err
 	}
 
 	filter := c.fvInterceptor.filter