asulwer / RulesEngine

Rules Engine with extensive Dynamic expression support
MIT License
26 stars 1 forks source link

EnableExceptionAsErrorMessage = false does not actually throw exception, but ignores/supresses it #65

Open rklec opened 3 months ago

rklec commented 3 months ago

STR

Have any invalid rule or one that somehow else throws an exception.

And have the settings configured as such:

      var engineSettings = new ReSettings()
        {
            CustomActions = // ...
            CustomTypes = // ...
            EnableExceptionAsErrorMessage = false
        };

        new RulesEngine.RulesEngine(mailWorkflows, engineSettings);     

What should happen

EnableExceptionAsErrorMessage = false is explained on https://microsoft.github.io/RulesEngine/#settings should do the following:

Otherwise [= if false], throws an exception. This setting is only applicable if IgnoreException is set to false.

Notably IgnoreException is not touched, i.e. it is (as per https://microsoft.github.io/RulesEngine/#settings aka the default set to false)

What happens

Errors are silently ignored.

More information

I have also indicated this in https://github.com/microsoft/RulesEngine/issues/623, see this for a practical example.

System

    <TargetFramework>net8.0</TargetFramework>
<!-- ... -->
    <PackageReference Include="RulesEngine" Version="5.0.3" /> 

aka .NET 8

Windows 10


copied/transferred from https://github.com/microsoft/RulesEngine/issues/624 (more details there)

RenanCarlosPereira commented 3 months ago

hey @rklec

we have some tests to cover this here:

are they valid?

please take a look and check if that make sense.

rklec commented 3 months ago

Well as far as I see they use the file rules9.json which only is a "traditional" workflow. In my case it is an action workflow,l maybe that makes a difference (here) (too).

asulwer commented 3 months ago

@rklec Those two properties work together.

var workflows = new Workflow[] {
    new() {
        WorkflowName = "Test Workflow1",
        Rules = new List<Rule> {
            new() {
                RuleName = "Test Rule1",
                SuccessEvent = "Count is less",
                ErrorMessage = "Over Expected",
                Expression = "counts < 3"
            },
            new() {
                RuleName = "Test Rule2",
                SuccessEvent = "Count is more",
                ErrorMessage = "Under Expected",
                Expression = "count > 3"
            }
        }
    }
};

Notice the Expression for each of the two rules. one contains count and the other counts. one should fail if input is count as follows

var settings = new ReSettings {
    //IgnoreException = true,
    EnableExceptionAsErrorMessage = false
};

var bre = new RulesEngine.RulesEngine(workflows, settings);

var inputs = new RuleParameter[] {new("input1", new {count = 1})};
asulwer commented 3 months ago

@rklec using actions the results are not the same. regardless of how EnableExceptionAsErrorMessage is set the exception is formatted and stored in action->exception, no runtime exception ever occurs

var workflows = new Workflow[] {
   new() {
        WorkflowName = "Test Workflow4",
        Rules = new List<Rule> {
            new() {
                RuleName = "Test Rule",
                Expression = "1 == 1",
                Actions = new RuleActions {
                    OnSuccess = new ActionInfo {
                        Name = "OutputExpression",
                        Context = new Dictionary<string, object> {{"expression", "counts"}}
                    }
                }
            }
        }
    }
};

var settings = new ReSettings {
    IgnoreException = true,
    EnableExceptionAsErrorMessage = true
};

var bre = new RulesEngine.RulesEngine(workflows, settings);

var inputs = new RuleParameter[] {new("input1", new {count = 1})};
asulwer commented 3 months ago

i believe the original author was removing all usage of Rule->Expression in favor of Rule->Actions

rklec commented 3 months ago

AFAIK you cannot use them together, can you? In any case, yes it's action rules, only.

using actions the results are not the same. regardless of how EnableExceptionAsErrorMessage is set the exception is formatted and stored in action->exception, no runtime exception ever occurs

In case I understand it correctly, that's exactly the issue I am facing, After all, why is it though? Why can EnableExceptionAsErrorMessage=false not also throw in a action? In any case, IMHO it's bad that this behavior is not documented. It should not matter what workflow type I am using, all settings should be applied, unless one setting says it does not apply. So at the very least, the doc needs improvement here.

EnableExceptionAsErrorMessage

  • true = runtime exception occurs
  • false = RuleResultTree->ExceptionMessage changes to formatted exception and no runtime exception

You meant to inverse true and false, did not you? (If so, maybe an inversed ThrowException property would have been the clearer API.

asulwer commented 3 months ago

@rklec those properties affect a normal workflow without actions. it is indeed an issue that actions only format the exception and not have the option to throw it as well. i will look into adjusting that for the next release, next couple of days.

leaving this issue open so it can be referenced with the solution

rklec commented 3 months ago

Also asked a related question about this, because it totally confuses me how many places there are to store some exceptions (or exception messages): https://github.com/microsoft/RulesEngine/discussions/628

asulwer commented 3 months ago

the code has loads of areas that need improvements. i am working on major changes. in my latest tests i went from 45 minute runtime to less than a minute, just with a couple of changes, major changes. coming soon