j-easy / easy-rules

The simple, stupid rules engine for Java
https://github.com/j-easy/easy-rules/wiki
MIT License
4.92k stars 1.06k forks source link

Swallowed exception in rule engine #135

Closed poluektik closed 6 years ago

poluektik commented 6 years ago

In case an action in the Rule throws an exception, the DefaultRuleEngine will swallow the exception and program will proceed its execution

following code reproduces the issue:

@Rule(name = "rule")
public class SomeRule {

    public static void main(String[] args) {
        // create a rules engine and fire rules on known facts
        RulesEngineParameters parameters = new RulesEngineParameters().
                skipOnFirstAppliedRule(true).
                skipOnFirstFailedRule(true);

        DefaultRulesEngine rulesEngine = new DefaultRulesEngine(parameters);

        Rules rules = new Rules();
        rules.register(new SomeRule()); //1

        Facts facts = new Facts();
        facts.put("fact", new StringBuilder("value"));
        try {
            rulesEngine.fire(rules, facts);
        } catch (Exception e) {
            System.out.println("gotcha");
        }
        System.out.println("end");
    }

    @Condition
    public boolean condition(@Fact("fact") StringBuilder builder) {
        if (builder != null)
            return true;
        return false;
    }

    @Action(order = 1)
    public void fire(Facts facts) throws FailedToOptimizeSummaryLineException {
        throw new FailedToOptimizeSummaryLineException("The Summary shouldn't have reach so far, there is nothing to optimize without any spent of events");
    }
}
poluektik commented 6 years ago

I'll add code example and full error stack, then reopen the issue

fmbenhassine commented 6 years ago

Good idea, that may help.

Thanks upfront!

poluektik commented 6 years ago

hi @benas can you suggest a workaround?

fmbenhassine commented 6 years ago

Hi,

Thank you for the code example.

This is actually how the engine is designed to work. If you look at the signature of the fire method, you see that it does not declare throwing exceptions. The exception is not swallowed, it is wrapped in a InvocationTargetException since the action method is invoked via reflection. You can use a RuleListener to get notified and react on the exception.

Kr Mahmoud

poluektik commented 6 years ago

Thanks for the tip, but it won't help. Our system designed to propagate exceptions, with easy-rules I'll have to maintain sort of status codes to notify callers.

closing the bug.

tsjoberg commented 6 years ago

FYI - I had a similar issue and followed along an example found here: https://stackoverflow.com/questions/39719370/how-to-wrap-checked-exceptions-but-keep-the-original-runtime-exceptions-in-java

This allowed me to rethrow the runtime exception accordingly and let the rest of the exception handlers operate as normal.

@benas would it make sense to implement something like this in the default rules listener to do this?

fmbenhassine commented 6 years ago

This is not an issue. Rules can throw exceptions but not the engine. For the record, drools has the same in WorkingMemory.fireAllRules API which does not throw exceptions.

This is how rules engines are designed to work. Simply because if two rules throw an exception, which one the engine should rethrow?

A rule throwing an exception can be seen as a new fact that may trigger other rules. For example, applying a business rule on a non existent customer may add a new fact "customer not existent" to the set of facts which in turn may or may not trigger subsequent rules. So it's not the engine's role to decide which exception should be thrown or not, the engine's sole role is to apply rules according to the given strategy.