diff --git a/CHANGELOG.md b/CHANGELOG.md index 437dc4395..315d26089 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ - [#1400](https://github.com/influxdata/kapacitor/issues/1400): Allow for `.yml` file extensions in `define-topic-handler` - [#1402](https://github.com/influxdata/kapacitor/pull/1402): Fix http server error logging. - [#1500](https://github.com/influxdata/kapacitor/pull/1500): Fix bugs with stopping running UDF agent. +- [#1470](https://github.com/influxdata/kapacitor/pull/1470): Fix error messages for missing fields which are arguments to functions are not clear ## v1.3.1 [2017-06-02] diff --git a/tick/ast/node.go b/tick/ast/node.go index 61b0cb1eb..a376f9e9f 100644 --- a/tick/ast/node.go +++ b/tick/ast/node.go @@ -236,6 +236,7 @@ func newBool(p position, text string, c *CommentNode) (*BoolNode, error) { func (n *BoolNode) String() string { return fmt.Sprintf("BoolNode@%v{%v}%v", n.position, n.Bool, n.Comment) } + func (n *BoolNode) Format(buf *bytes.Buffer, indent string, onNewLine bool) { if n.Comment != nil { n.Comment.Format(buf, indent, onNewLine) diff --git a/tick/stateful/eval_binary_node.go b/tick/stateful/eval_binary_node.go index 489dfbde4..ee0a0c7c2 100644 --- a/tick/stateful/eval_binary_node.go +++ b/tick/stateful/eval_binary_node.go @@ -117,6 +117,11 @@ func NewEvalBinaryNode(node *ast.BinaryNode) (*EvalBinaryNode, error) { return b, nil } +func (n *EvalBinaryNode) String() string { + + return fmt.Sprintf("%s %s %s", n.leftEvaluator, n.operator, n.rightEvaluator) +} + func (n *EvalBinaryNode) Type(scope ReadOnlyScope) (ast.ValueType, error) { if n.constReturnType == ast.InvalidType { var err error diff --git a/tick/stateful/eval_bool_node.go b/tick/stateful/eval_bool_node.go index f60eb788a..bc93ab3d0 100644 --- a/tick/stateful/eval_bool_node.go +++ b/tick/stateful/eval_bool_node.go @@ -1,6 +1,7 @@ package stateful import ( + "fmt" "regexp" "time" @@ -11,6 +12,10 @@ type EvalBoolNode struct { Node *ast.BoolNode } +func (n *EvalBoolNode) String() string { + return fmt.Sprintf("%v", n.Node.Bool) +} + func (n *EvalBoolNode) Type(scope ReadOnlyScope) (ast.ValueType, error) { return ast.TBool, nil } diff --git a/tick/stateful/eval_duration_node.go b/tick/stateful/eval_duration_node.go index cbd45adf7..808a6ecf6 100644 --- a/tick/stateful/eval_duration_node.go +++ b/tick/stateful/eval_duration_node.go @@ -1,6 +1,7 @@ package stateful import ( + "fmt" "regexp" "time" @@ -11,6 +12,10 @@ type EvalDurationNode struct { Duration time.Duration } +func (n *EvalDurationNode) String() string { + return fmt.Sprintf("%v", n.Duration) +} + func (n *EvalDurationNode) Type(scope ReadOnlyScope) (ast.ValueType, error) { return ast.TDuration, nil } diff --git a/tick/stateful/eval_float_node.go b/tick/stateful/eval_float_node.go index 8e7098021..e45d7fe54 100644 --- a/tick/stateful/eval_float_node.go +++ b/tick/stateful/eval_float_node.go @@ -1,6 +1,7 @@ package stateful import ( + "fmt" "regexp" "time" @@ -11,6 +12,10 @@ type EvalFloatNode struct { Float64 float64 } +func (n *EvalFloatNode) String() string { + return fmt.Sprintf("%v", n.Float64) +} + func (n *EvalFloatNode) Type(scope ReadOnlyScope) (ast.ValueType, error) { return ast.TFloat, nil } diff --git a/tick/stateful/eval_function_node.go b/tick/stateful/eval_function_node.go index 40471ee8a..c702c8447 100644 --- a/tick/stateful/eval_function_node.go +++ b/tick/stateful/eval_function_node.go @@ -7,6 +7,7 @@ import ( "time" "github.com/influxdata/kapacitor/tick/ast" + "github.com/pkg/errors" ) type EvalFunctionNode struct { @@ -32,6 +33,15 @@ func NewEvalFunctionNode(funcNode *ast.FunctionNode) (*EvalFunctionNode, error) return evalFuncNode, nil } +func (n *EvalFunctionNode) String() string { + args := []string{} + for _, argEvaluator := range n.argsEvaluators { + args = append(args, fmt.Sprintf("%s", argEvaluator)) + } + + return n.funcName + "(" + strings.Join(args, ", ") + ")" +} + func (n *EvalFunctionNode) Type(scope ReadOnlyScope) (ast.ValueType, error) { f := lookupFunc(n.funcName, builtinFuncs, scope) if f == nil { @@ -49,12 +59,33 @@ func (n *EvalFunctionNode) Type(scope ReadOnlyScope) (ast.ValueType, error) { } if gotLen, expLen := len(n.argsEvaluators), len(domain); gotLen > expLen { - return ast.InvalidType, ErrWrongFuncSignature{Name: n.funcName, DomainProvided: domain, Func: f} + err := ErrWrongFuncSignature{Name: n.funcName, DomainProvided: domain, Func: f} + return ast.InvalidType, errors.Wrapf(err, "too many arguments provided") } retType, ok := signature[domain] if !ok { - return ast.InvalidType, ErrWrongFuncSignature{Name: n.funcName, DomainProvided: domain, Func: f} + args := []string{} + missing := []string{} + for i, argEvaluator := range n.argsEvaluators { + t, err := argEvaluator.Type(scope) + if err != nil { + // This should never happen + return ast.InvalidType, fmt.Errorf("Failed to handle %v argument: %v", i+1, err) + } + + if t == ast.TMissing { + missing = append(missing, fmt.Sprintf("%s", argEvaluator)) + } + + args = append(args, fmt.Sprintf("%s", argEvaluator)) + } + + if len(missing) > 0 { + return ast.InvalidType, ErrMissingType{Name: n.funcName, Args: missing, Scope: scope.References()} + } + + return ast.InvalidType, ErrWrongFuncSignature{Name: n.funcName, ArgLiterals: args, DomainProvided: domain, Func: f} } return retType, nil diff --git a/tick/stateful/eval_int_node.go b/tick/stateful/eval_int_node.go index b2816e2ec..d99857711 100644 --- a/tick/stateful/eval_int_node.go +++ b/tick/stateful/eval_int_node.go @@ -1,6 +1,7 @@ package stateful import ( + "fmt" "regexp" "time" @@ -11,6 +12,10 @@ type EvalIntNode struct { Int64 int64 } +func (n *EvalIntNode) String() string { + return fmt.Sprintf("%v", n.Int64) +} + func (n *EvalIntNode) Type(scope ReadOnlyScope) (ast.ValueType, error) { return ast.TInt, nil } diff --git a/tick/stateful/eval_lambda_node.go b/tick/stateful/eval_lambda_node.go index 20a62a56e..533eb14a5 100644 --- a/tick/stateful/eval_lambda_node.go +++ b/tick/stateful/eval_lambda_node.go @@ -28,6 +28,10 @@ func NewEvalLambdaNode(lambda *ast.LambdaNode) (*EvalLambdaNode, error) { }, nil } +func (n *EvalLambdaNode) String() string { + return fmt.Sprintf("%s", n.nodeEvaluator) +} + func (n *EvalLambdaNode) Type(scope ReadOnlyScope) (ast.ValueType, error) { if n.constReturnType == ast.InvalidType { // We are dynamic and we need to figure out our type diff --git a/tick/stateful/eval_reference_node.go b/tick/stateful/eval_reference_node.go index 0e6e9f44c..115b794e3 100644 --- a/tick/stateful/eval_reference_node.go +++ b/tick/stateful/eval_reference_node.go @@ -25,6 +25,10 @@ func (n *EvalReferenceNode) getReferenceValue(scope *Scope) (interface{}, error) return value, nil } +func (n *EvalReferenceNode) String() string { + return "\"" + n.Node.Reference + "\"" +} + func (n *EvalReferenceNode) Type(scope ReadOnlyScope) (ast.ValueType, error) { value, err := n.getReferenceValue(scope.(*Scope)) if err != nil { diff --git a/tick/stateful/eval_regex_node.go b/tick/stateful/eval_regex_node.go index 9dce3e397..558a0bb4c 100644 --- a/tick/stateful/eval_regex_node.go +++ b/tick/stateful/eval_regex_node.go @@ -11,6 +11,10 @@ type EvalRegexNode struct { Node *ast.RegexNode } +func (n *EvalRegexNode) String() string { + return n.Node.Literal +} + func (n *EvalRegexNode) Type(scope ReadOnlyScope) (ast.ValueType, error) { return ast.TRegex, nil } diff --git a/tick/stateful/eval_string_node.go b/tick/stateful/eval_string_node.go index 4dc24ec26..35a8bfd98 100644 --- a/tick/stateful/eval_string_node.go +++ b/tick/stateful/eval_string_node.go @@ -11,6 +11,10 @@ type EvalStringNode struct { Node *ast.StringNode } +func (n *EvalStringNode) String() string { + return n.Node.Literal +} + func (n *EvalStringNode) Type(scope ReadOnlyScope) (ast.ValueType, error) { return ast.TString, nil } diff --git a/tick/stateful/eval_unary_node.go b/tick/stateful/eval_unary_node.go index 2e2bac3e4..8c231332c 100644 --- a/tick/stateful/eval_unary_node.go +++ b/tick/stateful/eval_unary_node.go @@ -9,6 +9,7 @@ import ( ) type EvalUnaryNode struct { + operator ast.TokenType nodeEvaluator NodeEvaluator constReturnType ast.ValueType } @@ -24,6 +25,7 @@ func NewEvalUnaryNode(unaryNode *ast.UnaryNode) (*EvalUnaryNode, error) { } return &EvalUnaryNode{ + operator: unaryNode.Operator, nodeEvaluator: nodeEvaluator, constReturnType: getConstantNodeType(unaryNode), }, nil @@ -33,6 +35,10 @@ func isValidUnaryOperator(operator ast.TokenType) bool { return operator == ast.TokenNot || operator == ast.TokenMinus } +func (n *EvalUnaryNode) String() string { + return fmt.Sprintf("%s%s", n.operator, n.nodeEvaluator) +} + func (n *EvalUnaryNode) Type(scope ReadOnlyScope) (ast.ValueType, error) { if n.constReturnType == ast.InvalidType { // We are dynamic and we need to figure out our type diff --git a/tick/stateful/functions.go b/tick/stateful/functions.go index 066f5288c..a1cf3b6de 100644 --- a/tick/stateful/functions.go +++ b/tick/stateful/functions.go @@ -23,15 +23,65 @@ const ( maxArgs = 4 ) +type ErrMissingType struct { + Name string + Args []string + Scope []string +} + +func (e ErrMissingType) Error() string { + s := "Cannot call function \"%s\" argument %s is missing, values in scope are [%s]" + if len(e.Args) > 1 { + s = "Cannot call function \"%s\" arguments %s are missing, values in scope are [%s]" + } + + // remove missing values from scope + for _, a := range e.Args { + e.Scope = removeElement(e.Scope, a) + } + + return fmt.Sprintf(s, e.Name, strings.Join(e.Args, ", "), strings.Join(e.Scope, ", ")) +} + +func removeElement(xs []string, el string) []string { + for i, x := range xs { + if x == el { + xs = append(xs[:i], xs[i+1:]...) + break + } + } + return xs +} + type ErrWrongFuncSignature struct { Name string DomainProvided Domain + ArgLiterals []string Func Func } func (e ErrWrongFuncSignature) Error() string { - return fmt.Sprintf("Cannot call function \"%s\" with args signature %s, available signatures are %s.", - e.Name, e.DomainProvided, FuncDomains(e.Func)) + var argStringer fmt.Stringer = &argDomain{args: e.ArgLiterals, domain: e.DomainProvided} + if e.ArgLiterals == nil { + argStringer = e.DomainProvided + } + return fmt.Sprintf("Cannot call function \"%s\" with args %s, available signatures are %s.", + e.Name, argStringer, FuncDomains(e.Func)) +} + +type argDomain struct { + args []string + domain Domain +} + +func (a *argDomain) String() string { + input := []string{} + for j, el := range a.args { + t := a.domain[j] + input = append(input, fmt.Sprintf("%s: %s", el, t)) + } + + return "(" + strings.Join(input, ",") + ")" } var ErrNotFloat = errors.New("value is not a float") diff --git a/tick/stateful/node_evaluator.go b/tick/stateful/node_evaluator.go index 7a15663ce..d0eb2a868 100644 --- a/tick/stateful/node_evaluator.go +++ b/tick/stateful/node_evaluator.go @@ -22,6 +22,7 @@ func (e ErrTypeGuardFailed) Error() string { type ReadOnlyScope interface { Get(name string) (interface{}, error) + References() []string DynamicFunc(name string) *DynamicFunc } diff --git a/tick/stateful/scope.go b/tick/stateful/scope.go index 7a7ffee78..f47a9ffee 100644 --- a/tick/stateful/scope.go +++ b/tick/stateful/scope.go @@ -44,6 +44,14 @@ func NewScope() *Scope { } } +func (s *Scope) References() []string { + ks := []string{} + for k := range s.variables { + ks = append(ks, "\""+k+"\"") + } + return ks +} + // Set defines a name -> value pairing in the scope. func (s *Scope) Set(name string, value interface{}) { s.variables[name] = value