feat: make expression package return ErrExpressionResolution when res…

…olution fails (#2292)

* make returned error a ErrExpressionResolution

* simplify error messages when expression resolution fails

* make sure all errors when unwrapped, don't start with "resolution error:"

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

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

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,15 +96,15 @@ 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()
 
 	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,26 +249,27 @@ 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
 }
 
 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())
 	}
 
 	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)

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

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