deliveredtechnologies / rulebook

100% Java, Lambda Enabled, Lightweight Rules Engine with a Simple and Intuitive DSL
http://www.deliveredtechnologies.com
Apache License 2.0
716 stars 124 forks source link

rule book runner incorrectly eats exceptions so buggy code is not caught. #184

Closed jimmymain closed 3 years ago

jimmymain commented 4 years ago

The below code that sets the @Result field in a POJO class logs the exception Unable to set result field in...

But the invalid result is returned and the rule runner does not indicate in any way that it is broken. The exception should be propagated, not just logged. At least add a state variable or something to the rule engine so that when it's complete we know it's broken.

I understand that all the pojo rules should have the same @Result return type, but the rule engine should not merrily carry on if the wrong type is in the rule.

log by all means, but then throw var4;

Can I do a PR for it?

    public void setResult(Result result) {
        this._rule.setResult(result);
        AnnotationUtils.getAnnotatedField(com.deliveredtechnologies.rulebook.annotation.Result.class, this._pojoRule.getClass()).ifPresent((field) -> {
            field.setAccessible(true);

            try {
                if (result.getValue() != null && (result.getValue().getClass() != Object.class || field.getType().isAssignableFrom(result.getValue().getClass()))) {
                    field.set(this._pojoRule, result.getValue());
                }
            } catch (Exception var4) {
                LOGGER.error("Unable to set @Result field in " + this._pojoRule.getClass(), var4);
            }

        });
    }

all my POJO classes are ruleChainAction = RuleChainActionType.ERROR_ON_FAILURE so I am expecting an exception.

Clayton7510 commented 4 years ago

Looks like a bug on the face of it. A PR would be welcome. Thanks.

jimmymain commented 4 years ago

https://github.com/deliveredtechnologies/rulebook/pull/186