Let runtime exceptions in conditions bubble up and handle them in the engine

Issue #211

easy-rules-core/src/main/java/org/jeasy/rules/api/RuleListener.java

@@ -50,6 +50,15 @@ public interface RuleListener {
      */
     default void afterEvaluate(Rule rule, Facts facts, boolean evaluationResult) { }
 
+    /**
+     * Triggered on condition evaluation error due to any runtime exception.
+     *
+     * @param rule that has been evaluated
+     * @param facts known while evaluating the rule
+     * @param exception that happened while attempting to evaluate the condition.
+     */
+    default void onEvaluationError(Rule rule, Facts facts, Exception exception) { }
+
     /**
      * Triggered before the execution of a rule.
      *

easy-rules-core/src/main/java/org/jeasy/rules/core/DefaultRulesEngine.java

@@ -88,7 +88,19 @@ public final class DefaultRulesEngine extends AbstractRulesEngine {
                     name);
                 continue;
             }
-            if (rule.evaluate(facts)) {
+            boolean evaluationResult = false;
+            try {
+                evaluationResult = rule.evaluate(facts);
+            } catch (RuntimeException exception) {
+                LOGGER.error("Rule '" + name + "' evaluated with error", exception);
+                triggerListenersOnEvaluationError(rule, facts, exception);
+                // give the option to either skip next rules on evaluation error or continue by considering the evaluation error as false
+                if (parameters.isSkipOnFirstNonTriggeredRule()) {
+                    LOGGER.debug("Next rules will be skipped since parameter skipOnFirstNonTriggeredRule is set");
+                    break;
+                }
+            }
+            if (evaluationResult) {
                 LOGGER.debug("Rule '{}' triggered", name);
                 triggerListenersAfterEvaluate(rule, facts, true);
                 try {
@@ -178,6 +190,10 @@ public final class DefaultRulesEngine extends AbstractRulesEngine {
         ruleListeners.forEach(ruleListener -> ruleListener.afterEvaluate(rule, facts, evaluationResult));
     }
 
+    private void triggerListenersOnEvaluationError(Rule rule, Facts facts, Exception exception) {
+        ruleListeners.forEach(ruleListener -> ruleListener.onEvaluationError(rule, facts, exception));
+    }
+
     private void triggerListenersBeforeRules(Rules rule, Facts facts) {
         rulesEngineListeners.forEach(rulesEngineListener -> rulesEngineListener.beforeEvaluate(rule, facts));
     }

easy-rules-core/src/main/java/org/jeasy/rules/core/RuleProxy.java

@@ -121,12 +121,13 @@ public class RuleProxy implements InvocationHandler {
             List<Object> actualParameters = getActualParameters(conditionMethod, facts);
             return conditionMethod.invoke(target, actualParameters.toArray()); // validated upfront
         } catch (NoSuchFactException e) {
-            LOGGER.error("Rule '{}' has been evaluated to false due to a declared but missing fact '{}' in {}",
+            LOGGER.warn("Rule '{}' has been evaluated to false due to a declared but missing fact '{}' in {}",
                     getTargetClass().getName(), e.getMissingFact(), facts);
             return false;
         } catch (IllegalArgumentException e) {
-            String error = "Types of injected facts in method '%s' in rule '%s' do not match parameters types";
-            throw new RuntimeException(format(error, conditionMethod.getName(), getTargetClass().getName()), e);
+            LOGGER.warn("Types of injected facts in method '{}' in rule '{}' do not match parameters types",
+                    conditionMethod.getName(), getTargetClass().getName(), e);
+            return false;
         }
     }
 

easy-rules-core/src/test/java/org/jeasy/rules/core/FactInjectionTest.java

@@ -81,19 +81,20 @@ public class FactInjectionTest {
         assertThat(weatherRule.isExecuted()).isTrue();
     }
 
-    @Test(expected = RuntimeException.class)
-    public void whenFactTypeDoesNotMatchParameterType_thenShouldThrowRuntimeException() {
+    @Test
+    public void whenFactTypeDoesNotMatchParameterType_thenTheRuleShouldNotBeExecuted() {
         // Given
         Facts facts = new Facts();
         facts.put("age", "foo");
-        Rules rules = new Rules(new AgeRule());
+        AgeRule ageRule = new AgeRule();
+        Rules rules = new Rules(ageRule);
         RulesEngine rulesEngine = new DefaultRulesEngine();
 
         // When
         rulesEngine.fire(rules, facts);
 
         // Then
-        // expected exception
+        assertThat(ageRule.isExecuted()).isFalse();
     }
 
     @Test

easy-rules-mvel/src/main/java/org/jeasy/rules/mvel/MVELCondition.java

@@ -27,8 +27,6 @@ import org.jeasy.rules.api.Condition;
 import org.jeasy.rules.api.Facts;
 import org.mvel2.MVEL;
 import org.mvel2.ParserContext;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import java.io.Serializable;
 
@@ -39,9 +37,6 @@ import java.io.Serializable;
  */
 public class MVELCondition implements Condition {
 
-    private static final Logger LOGGER = LoggerFactory.getLogger(MVELCondition.class);
-
-    private String expression;
     private Serializable compiledExpression;
 
     /**
@@ -50,7 +45,6 @@ public class MVELCondition implements Condition {
      * @param expression the condition written in expression language
      */
     public MVELCondition(String expression) {
-        this.expression = expression;
         compiledExpression = MVEL.compileExpression(expression);
     }
 
@@ -61,17 +55,11 @@ public class MVELCondition implements Condition {
      * @param parserContext the MVEL parser context
      */
     public MVELCondition(String expression, ParserContext parserContext) {
-        this.expression = expression;
         compiledExpression = MVEL.compileExpression(expression, parserContext);
     }
 
     @Override
     public boolean evaluate(Facts facts) {
-        try {
-            return (boolean) MVEL.executeExpression(compiledExpression, facts.asMap());
-        } catch (Exception e) {
-            LOGGER.error("Unable to evaluate expression: '" + expression + "' on facts: " + facts, e);
-            return false;
-        }
+        return (boolean) MVEL.executeExpression(compiledExpression, facts.asMap());
     }
 }

easy-rules-mvel/src/test/java/org/jeasy/rules/mvel/MVELConditionTest.java

@@ -46,8 +46,9 @@ public class MVELConditionTest {
         assertThat(evaluationResult).isTrue();
     }
 
-    @Test
-    public void whenDeclaredFactIsNotPresent_thenShouldReturnFalse() {
+    // Note this behaviour is different in SpEL, where a missing fact is silently ignored and returns false
+    @Test(expected = RuntimeException.class)
+    public void whenDeclaredFactIsNotPresent_thenShouldThrowRuntimeException() {
         // given
         Condition isHot = new MVELCondition("temperature > 30");
         Facts facts = new Facts();
@@ -56,7 +57,7 @@ public class MVELConditionTest {
         boolean evaluationResult = isHot.evaluate(facts);
 
         // then
-        assertThat(evaluationResult).isFalse();
+        // expected exception
     }
 
     @Test

easy-rules-spel/src/main/java/org/jeasy/rules/spel/SpELCondition.java

@@ -25,8 +25,6 @@ package org.jeasy.rules.spel;
 
 import org.jeasy.rules.api.Condition;
 import org.jeasy.rules.api.Facts;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import org.springframework.expression.BeanResolver;
 import org.springframework.expression.Expression;
@@ -46,11 +44,8 @@ import org.springframework.expression.spel.support.StandardEvaluationContext;
  */
 public class SpELCondition implements Condition {
 
-    private static final Logger LOGGER = LoggerFactory.getLogger(SpELCondition.class);
-
     private final ExpressionParser parser = new SpelExpressionParser();
 
-    private String expression;
     private Expression compiledExpression;
     private BeanResolver beanResolver;
 
@@ -60,7 +55,6 @@ public class SpELCondition implements Condition {
      * @param expression the condition written in expression language
      */
     public SpELCondition(String expression) {
-        this.expression = expression;
         compiledExpression = parser.parseExpression(expression);
     }
 
@@ -71,7 +65,6 @@ public class SpELCondition implements Condition {
      * @param beanResolver  the bean resolver used to resolve bean references
      */
     public SpELCondition(String expression, BeanResolver beanResolver) {
-        this.expression = expression;
         this.beanResolver = beanResolver;
         compiledExpression = parser.parseExpression(expression);
     }
@@ -83,7 +76,6 @@ public class SpELCondition implements Condition {
      * @param parserContext the SpEL parser context
      */
     public SpELCondition(String expression, ParserContext parserContext) {
-        this.expression = expression;
         compiledExpression = parser.parseExpression(expression, parserContext);
     }
 
@@ -95,24 +87,18 @@ public class SpELCondition implements Condition {
      * @param parserContext the SpEL parser context
      */
     public SpELCondition(String expression, ParserContext parserContext, BeanResolver beanResolver) {
-        this.expression = expression;
         this.beanResolver = beanResolver;
         compiledExpression = parser.parseExpression(expression, parserContext);
     }
 
     @Override
     public boolean evaluate(Facts facts) {
-        try {
-            StandardEvaluationContext context = new StandardEvaluationContext();
-            context.setRootObject(facts.asMap());
-            context.setVariables(facts.asMap());
-            if (beanResolver != null) {
-                context.setBeanResolver(beanResolver);
-            }
-            return  compiledExpression.getValue(context, Boolean.class);
-        } catch (Exception e) {
-            LOGGER.error("Unable to evaluate expression: '" + expression + "' on facts: " + facts, e);
-            return false;
+        StandardEvaluationContext context = new StandardEvaluationContext();
+        context.setRootObject(facts.asMap());
+        context.setVariables(facts.asMap());
+        if (beanResolver != null) {
+            context.setBeanResolver(beanResolver);
         }
+        return compiledExpression.getValue(context, Boolean.class);
     }
 }

easy-rules-spel/src/test/java/org/jeasy/rules/spel/SpELConditionTest.java

@@ -51,6 +51,7 @@ public class SpELConditionTest {
         assertThat(evaluationResult).isTrue();
     }
 
+    // Note this behaviour is different in MVEL, where a missing fact yields an exception
     @Test
     public void whenDeclaredFactIsNotPresent_thenShouldReturnFalse() {
         // given

easy-rules-support/src/test/java/org/jeasy/rules/support/ConditionalRuleGroupTest.java

@@ -167,7 +167,7 @@ public class ConditionalRuleGroupTest {
         conditionalRuleGroup.addRule(new MyOtherRule(0));// same priority as conditionalRule
         conditionalRuleGroup.addRule(new MyOtherRule(1));
         conditionalRuleGroup.addRule(new MyRule());
-        rulesEngine.fire(rules, facts);
+        conditionalRuleGroup.evaluate(facts);
     }
 
     @Test