Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert ConverterError to InvalidArgument in standard visibility parser #2437

Merged
merged 2 commits into from
Feb 2, 2022
Merged

Convert ConverterError to InvalidArgument in standard visibility parser #2437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1d6c51b
Convert ConverterError to InvalidArgument in standard visibility parser
alexshtin Jan 29, 2022
1cf5370
Fix unit test
alexshtin Feb 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions common/persistence/visibility/store/elasticsearch/visibility_store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
16 changes: 8 additions & 8 deletions common/persistence/visibility/store/elasticsearch/visibility_store_read_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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
}

Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"`
Expand Down Expand Up @@ -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() {
Expand Down
14 changes: 10 additions & 4 deletions common/persistence/visibility/store/query/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,13 @@ package query
import (
"errors"
"fmt"

"go.temporal.io/api/serviceerror"
)

type (
ConverterError struct {
text string
message string
}
)

Expand All @@ -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 {
Expand Down
10 changes: 8 additions & 2 deletions common/persistence/visibility/store/standard/converter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Expand Down