Skip to content
This repository has been archived by the owner on Mar 21, 2023. It is now read-only.

add parse error handler for precompute args failures #93

Merged
merged 4 commits into from Aug 25, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,35 @@
/**
* This file is part of Graylog Pipeline Processor.
*
* Graylog Pipeline Processor is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Graylog Pipeline Processor is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with Graylog Pipeline Processor. If not, see <http://www.gnu.org/licenses/>.
*/
package org.graylog.plugins.pipelineprocessor.ast.exceptions;

public class PrecomputeFailure extends RuntimeException {
private final String argumentName;

public PrecomputeFailure(String argumentName, Exception cause) {
super(cause);
this.argumentName = argumentName;
}

public String getArgumentName() {
return argumentName;
}

@Override
public String getMessage() {
return "Unable to pre-compute argument " + getArgumentName() + ": " + getCause().getMessage();
}
}
Expand Up @@ -18,6 +18,7 @@

import com.google.common.collect.ImmutableList;
import org.graylog.plugins.pipelineprocessor.EvaluationContext;
import org.graylog.plugins.pipelineprocessor.ast.exceptions.PrecomputeFailure;
import org.graylog.plugins.pipelineprocessor.ast.expressions.Expression;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -63,8 +64,8 @@ default void preprocessArgs(FunctionArgs args) {
args.setPreComputedValue(name, param.transform().apply(value));
}
} catch (Exception exception) {
log.warn("Unable to precompute argument value for " + name, exception);
throw exception;
log.debug("Unable to precompute argument value for " + name, exception);
throw new PrecomputeFailure(name, exception);
}
}

Expand Down
Expand Up @@ -33,7 +33,7 @@ public abstract class TimezoneAwareFunction extends AbstractFunction<DateTime> {
public TimezoneAwareFunction() {
timeZoneParam = ParameterDescriptor
.string(TIMEZONE, DateTimeZone.class)
.transform(DateTimeZone::forID)
.transform(id -> DateTimeZone.forID(id.toUpperCase()))
.optional()
.description("The timezone to apply to the date, defaults to UTC")
.build();
Expand Down
Expand Up @@ -36,6 +36,7 @@
import org.graylog.plugins.pipelineprocessor.ast.Pipeline;
import org.graylog.plugins.pipelineprocessor.ast.Rule;
import org.graylog.plugins.pipelineprocessor.ast.Stage;
import org.graylog.plugins.pipelineprocessor.ast.exceptions.PrecomputeFailure;
import org.graylog.plugins.pipelineprocessor.ast.expressions.AndExpression;
import org.graylog.plugins.pipelineprocessor.ast.expressions.ArrayLiteralExpression;
import org.graylog.plugins.pipelineprocessor.ast.expressions.BinaryExpression;
Expand Down Expand Up @@ -68,6 +69,7 @@
import org.graylog.plugins.pipelineprocessor.parser.errors.IncompatibleIndexType;
import org.graylog.plugins.pipelineprocessor.parser.errors.IncompatibleType;
import org.graylog.plugins.pipelineprocessor.parser.errors.IncompatibleTypes;
import org.graylog.plugins.pipelineprocessor.parser.errors.InvalidFunctionArgument;
import org.graylog.plugins.pipelineprocessor.parser.errors.MissingRequiredParam;
import org.graylog.plugins.pipelineprocessor.parser.errors.NonIndexableType;
import org.graylog.plugins.pipelineprocessor.parser.errors.OptionalParametersMustBeNamed;
Expand Down Expand Up @@ -357,9 +359,15 @@ public void exitFunctionCall(RuleLangParser.FunctionCallContext ctx) {
}
}

final FunctionExpression expr = new FunctionExpression(
ctx.getStart(), new FunctionArgs(functionRegistry.resolveOrError(name), argsMap)
);
FunctionExpression expr;
try {
expr = new FunctionExpression(
ctx.getStart(), new FunctionArgs(functionRegistry.resolveOrError(name), argsMap)
);
} catch (PrecomputeFailure precomputeFailure) {
parseContext.addError(new InvalidFunctionArgument(ctx, function, precomputeFailure));
expr = new FunctionExpression(ctx.getStart(), new FunctionArgs(Function.ERROR_FUNCTION, argsMap));
}

log.trace("FUNC: ctx {} => {}", ctx, expr);
exprs.put(ctx, expr);
Expand Down
@@ -0,0 +1,50 @@
/**
* This file is part of Graylog Pipeline Processor.
*
* Graylog Pipeline Processor is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Graylog Pipeline Processor is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with Graylog Pipeline Processor. If not, see <http://www.gnu.org/licenses/>.
*/
package org.graylog.plugins.pipelineprocessor.parser.errors;

import com.fasterxml.jackson.annotation.JsonProperty;
import org.graylog.plugins.pipelineprocessor.ast.exceptions.PrecomputeFailure;
import org.graylog.plugins.pipelineprocessor.ast.functions.Function;
import org.graylog.plugins.pipelineprocessor.ast.functions.ParameterDescriptor;
import org.graylog.plugins.pipelineprocessor.parser.RuleLangParser;

public class InvalidFunctionArgument extends ParseError {
private final Function<?> function;
private final PrecomputeFailure failure;

public InvalidFunctionArgument(RuleLangParser.FunctionCallContext ctx,
Function<?> function,
PrecomputeFailure failure) {
super("invalid_function_argument", ctx);
this.function = function;
this.failure = failure;
}

@JsonProperty("reason")
@Override
public String toString() {
int paramPosition = 1;
for (ParameterDescriptor descriptor : function.descriptor().params()) {
if (descriptor.name().equals(failure.getArgumentName())) {
break;
}
paramPosition++;
}

return "Unable to pre-compute value for " + paramPosition + ". argument " + failure.getArgumentName() + " in call to function " + function.descriptor().name() + ": " + failure.getCause().getMessage();
Copy link

@hc4 hc4 Aug 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is should be "," before argument,right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually there should be "st", "nd", "rd", "th" there, but I was too lazy for an error message ;)

It's the argument position, since we don't highlight it yet.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe write:
return "Unable to pre-compute value for argument " + failure.getArgumentName() + " (pos. "+paramPosition +") in call..

Copy link
Member Author

@kroepke kroepke Aug 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I like the abbreviated ordinal better, so I got unlazy and implemented that. The error now reads:
Unable to pre-compute value for 1st argument timezone in call to function now_in_tz: The datetime zone id '123' is not recognised

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool 👍
Maybe put argument name in quotes for better readability?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for now I'll skip that, because then I would need to touch all the other errors, too.
We also don't know how we will continue with the editor in 2.2 so I'd like to leave it at this.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.
Anyway 3rd much better than 3. :)

}
}
Expand Up @@ -16,6 +16,7 @@
*/
package org.graylog.plugins.pipelineprocessor.parser;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Ordering;
Expand All @@ -31,17 +32,20 @@
import org.graylog.plugins.pipelineprocessor.ast.functions.ParameterDescriptor;
import org.graylog.plugins.pipelineprocessor.functions.conversion.LongConversion;
import org.graylog.plugins.pipelineprocessor.functions.conversion.StringConversion;
import org.graylog.plugins.pipelineprocessor.functions.dates.TimezoneAwareFunction;
import org.graylog.plugins.pipelineprocessor.functions.messages.HasField;
import org.graylog.plugins.pipelineprocessor.functions.messages.SetField;
import org.graylog.plugins.pipelineprocessor.functions.strings.RegexMatch;
import org.graylog.plugins.pipelineprocessor.parser.errors.IncompatibleArgumentType;
import org.graylog.plugins.pipelineprocessor.parser.errors.IncompatibleIndexType;
import org.graylog.plugins.pipelineprocessor.parser.errors.InvalidFunctionArgument;
import org.graylog.plugins.pipelineprocessor.parser.errors.NonIndexableType;
import org.graylog.plugins.pipelineprocessor.parser.errors.OptionalParametersMustBeNamed;
import org.graylog.plugins.pipelineprocessor.parser.errors.UndeclaredFunction;
import org.graylog.plugins.pipelineprocessor.parser.errors.UndeclaredVariable;
import org.graylog2.plugin.Message;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.junit.After;
import org.junit.Assert;
import org.junit.BeforeClass;
Expand Down Expand Up @@ -253,6 +257,27 @@ public FunctionDescriptor<Collection> descriptor() {
functions.put(SetField.NAME, new SetField());
functions.put(HasField.NAME, new HasField());
functions.put(RegexMatch.NAME, new RegexMatch());
functions.put("now_in_tz", new TimezoneAwareFunction() {
@Override
protected DateTime evaluate(FunctionArgs args, EvaluationContext context, DateTimeZone timezone) {
return DateTime.now(timezone);
}

@Override
protected String description() {
return "Now in the given timezone";
}

@Override
protected String getName() {
return "now_in_tz";
}

@Override
protected ImmutableList<ParameterDescriptor> params() {
return ImmutableList.of();
}
});
functionRegistry = new FunctionRegistry(functions);
}

Expand Down Expand Up @@ -467,6 +492,16 @@ public void indexedAccessWrongIndexType() {
}
}

@Test
public void invalidArgumentValue() {
try {
parser.parseRule(ruleForTest(), false);
} catch (ParseException e) {
assertEquals(1, e.getErrors().size());
assertEquals(InvalidFunctionArgument.class, Iterables.getOnlyElement(e.getErrors()).getClass());
}
}

public static class CustomObject {
private final String id;

Expand Down
@@ -0,0 +1,4 @@
rule "invalid arg"
when now_in_tz("123") // this isn't a valid tz
then
end