diff --git a/server/expression/data_store.go b/server/expression/data_store.go index 3ac62192c9..66827488fe 100644 --- a/server/expression/data_store.go +++ b/server/expression/data_store.go @@ -21,7 +21,12 @@ func (ds AttributeDataStore) Source() string { } func (ds AttributeDataStore) Get(name string) (string, error) { - return ds.Span.Attributes.Get(name), nil + value := ds.Span.Attributes.Get(name) + if value == "" { + return "", fmt.Errorf(`attribute "%s" not found`, name) + } + + return value, nil } type MetaAttributesDataStore struct { @@ -74,5 +79,5 @@ func (ds EnvironmentDataStore) Get(name string) (string, error) { } } - return "", nil + return "", fmt.Errorf(`environment variable "%s" not found`, name) } diff --git a/server/expression/errors.go b/server/expression/errors.go new file mode 100644 index 0000000000..8a994b018e --- /dev/null +++ b/server/expression/errors.go @@ -0,0 +1,33 @@ +package expression + +import ( + "errors" + "fmt" +) + +var ErrExpressionResolution error = errors.New("resolution error") + +type ResolutionError struct { + innerErr error +} + +func (e *ResolutionError) Error() string { + return fmt.Sprintf("%s: %s", ErrExpressionResolution.Error(), e.innerErr.Error()) +} + +func (e *ResolutionError) Is(target error) bool { + // all Resolution errors are ErrExpressionResolution + if errors.Is(target, ErrExpressionResolution) { + return true + } + + return errors.Is(e.innerErr, target) +} + +func (e *ResolutionError) Unwrap() error { + return e.innerErr +} + +func resolutionError(innerErr error) error { + return &ResolutionError{innerErr: innerErr} +} diff --git a/server/expression/executor.go b/server/expression/executor.go index cc20182cfa..34802ebe92 100644 --- a/server/expression/executor.go +++ b/server/expression/executor.go @@ -39,12 +39,12 @@ func (e Executor) Statement(statement string) (string, string, error) { leftValue, err := e.ResolveExpression(parsedStatement.Left) if err != nil { - return "", "", fmt.Errorf("could not parse left side expression: %w", err) + return "", "", err } rightValue, err := e.ResolveExpression(parsedStatement.Right) if err != nil { - return "", "", fmt.Errorf("could not parse left side expression: %w", err) + return "", "", err } // https://github.com/kubeshop/tracetest/issues/1203 @@ -96,7 +96,7 @@ func (e Executor) ResolveStatement(statement string) (string, error) { leftValue, err := e.ResolveExpression(parsedStatement.Left) if err != nil { - return "", fmt.Errorf("could not parse left side expression: %w", err) + return "", err } parsed = parsed + leftValue.String() @@ -104,7 +104,7 @@ func (e Executor) ResolveStatement(statement string) (string, error) { if parsedStatement.Right != nil { rightValue, err := e.ResolveExpression(parsedStatement.Right) if err != nil { - return "", fmt.Errorf("could not parse left side expression: %w", err) + return "", err } parsed = fmt.Sprintf("%s %s %s", parsed, parsedStatement.Comparator, rightValue.String()) @@ -131,14 +131,14 @@ func (e Executor) Expression(expression string) (value.Value, error) { func (e Executor) ResolveExpression(expr *Expr) (value.Value, error) { currentValue, err := e.resolveTerm(expr.Left) if err != nil { - return value.Nil, fmt.Errorf("could not resolve term: %w", err) + return value.Nil, err } if expr.Right != nil { for _, opTerm := range expr.Right { newValue, err := e.executeOperation(currentValue.Value(), opTerm) if err != nil { - return value.Nil, fmt.Errorf("could not execute operation: %w", err) + return value.Nil, err } currentValue = newValue @@ -235,7 +235,7 @@ func (e Executor) resolveAttribute(attribute *Attribute) (value.Value, error) { attributeDataStore := e.Stores["attr"] attributeValue, err := attributeDataStore.Get(attribute.Name()) if err != nil { - return value.Nil, fmt.Errorf("could not resolve attribute %s: %w", attribute.Name(), err) + return value.Nil, resolutionError(err) } return value.NewFromString(attributeValue), nil @@ -249,7 +249,7 @@ func (e Executor) resolveEnvironment(environment *Environment) (value.Value, err envDataStore := e.Stores["env"] envValue, err := envDataStore.Get(environment.Name()) if err != nil { - return value.Nil, fmt.Errorf("could not resolve variable %s: %w", environment.Name(), err) + return value.Nil, resolutionError(err) } return value.NewFromString(envValue), nil @@ -257,10 +257,11 @@ func (e Executor) resolveEnvironment(environment *Environment) (value.Value, err func (e Executor) resolveFunctionCall(functionCall *FunctionCall) (value.Value, error) { args := make([]types.TypedValue, 0, len(functionCall.Args)) - for i, arg := range functionCall.Args { + for index, arg := range functionCall.Args { functionValue, err := e.resolveTerm(arg) if err != nil { - return value.Nil, fmt.Errorf("could not execute function %s: invalid argument on index %d: %w", functionCall.Name, i, err) + newErr := fmt.Errorf("on argument %d of function %s: %w", index, functionCall.Name, err) + return value.Nil, resolutionError(newErr) } args = append(args, functionValue.Value()) @@ -268,7 +269,7 @@ func (e Executor) resolveFunctionCall(functionCall *FunctionCall) (value.Value, function, err := functions.DefaultRegistry().Get(functionCall.Name) if err != nil { - return value.Nil, fmt.Errorf("function %s doesn't exist", functionCall.Name) + return value.Nil, resolutionError(err) } typedValue, err := function.Invoke(args...) @@ -280,7 +281,9 @@ func (e Executor) resolveArray(array *Array) (value.Value, error) { for index, item := range array.Items { termValue, err := e.resolveTerm(item) if err != nil { - return value.Value{}, fmt.Errorf("could not resolve item at index %d: %w", index, err) + innerError := errors.Unwrap(err) + newErr := fmt.Errorf("at index %d of array: %w", index, innerError) + return value.Nil, resolutionError(newErr) } typedValues = append(typedValues, termValue.Value()) @@ -301,7 +304,7 @@ func (e Executor) executeOperation(left types.TypedValue, right *OpTerm) (value. operatorFunction, err := getOperationRegistry().Get(right.Operator) if err != nil { - return value.Nil, err + return value.Nil, resolutionError(err) } newValue, err := operatorFunction(left, rightValue.Value()) @@ -331,7 +334,7 @@ func (e Executor) executeFilter(input value.Value, filter *Filter) (value.Value, for _, arg := range filter.Args { resolvedArg, err := e.resolveTerm(arg) if err != nil { - return value.Value{}, err + return value.Nil, err } args = append(args, resolvedArg.Value().Value) diff --git a/server/expression/executor_test.go b/server/expression/executor_test.go index 0e223a5a14..725b04d3ba 100644 --- a/server/expression/executor_test.go +++ b/server/expression/executor_test.go @@ -1,21 +1,26 @@ package expression_test import ( + "errors" "fmt" + "strings" "testing" "github.com/kubeshop/tracetest/server/expression" "github.com/kubeshop/tracetest/server/model" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type executorTestCase struct { - Name string - Query string - ShouldPass bool + Name string + Query string + ShouldPass bool + ExpectedErrorMessage string AttributeDataStore expression.DataStore MetaAttributesDataStore expression.DataStore + EnvironmentDataStore expression.DataStore } func TestBasicExpressionExecution(t *testing.T) { @@ -405,19 +410,76 @@ func TestResolveStatementFilterExecution(t *testing.T) { executeResolveStatementTestCases(t, testCases) } +func TestFailureCases(t *testing.T) { + testCases := []executorTestCase{ + { + Name: "should_report_missing_environment_variable", + Query: `env:test = "abc"`, + ShouldPass: false, + ExpectedErrorMessage: `resolution error: environment variable "test" not found`, + + EnvironmentDataStore: expression.EnvironmentDataStore{ + Values: []model.EnvironmentValue{}, + }, + }, + { + Name: "should_report_missing_attribute", + Query: `attr:my_attribute = "abc"`, + ShouldPass: false, + ExpectedErrorMessage: `resolution error: attribute "my_attribute" not found`, + + AttributeDataStore: expression.AttributeDataStore{ + Span: model.Span{ + Attributes: model.Attributes{ + "attr1": "1", + "attr2": "2", + }, + }, + }, + }, + { + Name: "should_report_missing_filter", + Query: `"value" | missingFilter = "abc"`, + ShouldPass: false, + ExpectedErrorMessage: `resolution error: filter "missingFilter" not found`, + }, + { + Name: "should_report_problem_resolving_array_item", + Query: `["value", env:test, "anotherValue"] | get_index 0`, + ShouldPass: false, + ExpectedErrorMessage: `resolution error: at index 1 of array: environment variable "test" not found`, + + EnvironmentDataStore: expression.EnvironmentDataStore{ + Values: []model.EnvironmentValue{}, + }, + }, + } + + executeResolveStatementTestCases(t, testCases) +} + func executeResolveStatementTestCases(t *testing.T, testCases []executorTestCase) { for _, testCase := range testCases { t.Run(testCase.Name, func(t *testing.T) { executor := expression.NewExecutor( testCase.AttributeDataStore, testCase.MetaAttributesDataStore, + testCase.EnvironmentDataStore, ) left, err := executor.ResolveStatement(testCase.Query) debugMessage := fmt.Sprintf("left value: %s", left) if testCase.ShouldPass { assert.NoError(t, err, debugMessage) } else { - assert.Error(t, err, debugMessage) + require.Error(t, err, debugMessage) + if testCase.ExpectedErrorMessage != "" { + assert.Equal(t, testCase.ExpectedErrorMessage, err.Error()) + // all validation erros should be ErrExpressionResolution errors + assert.ErrorIs(t, err, expression.ErrExpressionResolution) + + errorMessageDoesntStartWithResolutionError := !strings.HasPrefix(errors.Unwrap(err).Error(), "resolution error:") + assert.True(t, errorMessageDoesntStartWithResolutionError) + } } }) } diff --git a/server/expression/filters.go b/server/expression/filters.go index 772b4c7a29..9a06bcbb87 100644 --- a/server/expression/filters.go +++ b/server/expression/filters.go @@ -21,7 +21,7 @@ var filterFunctions = map[string]filterFn{ func executeFilter(input value.Value, filterName string, args []string) (value.Value, error) { fn, found := filterFunctions[filterName] if !found { - return value.Value{}, fmt.Errorf("filter %s was not found", filterName) + return value.Nil, resolutionError(fmt.Errorf(`filter "%s" not found`, filterName)) } output, err := fn(input, args...)