openhab-core icon indicating copy to clipboard operation
openhab-core copied to clipboard

Allow to run rules using scheduler to avoid "Failed to execute rule ‘{}' with status '{}'" error path

Open OlegAndreych opened this issue 1 year ago • 6 comments

I propose to use org.openhab.core.automation.internal.TriggerHandlerCallbackImpl#getScheduler when calling scripts to resolve an issue with simultaneous executions, as it is done for "normal" rules execution path.

That's needed for a code reuse using UI scripts. Many rules are triggered with events with no known intervals and/or timing, so there's no sensible way to spread executions of these rules, so that they don't call scripts simultaneously.

An example of an issue can be found there: https://community.openhab.org/t/simultaneous-calling-of-script-problem/140087.

OlegAndreych avatar Jan 28 '24 16:01 OlegAndreych

I commented on the PR because I didn't see this issue at first. Here is a better place to have the discussion.

This might be a breaking change. Right now when one rule calls another rule, the call blocks until the called rule returns.

For example:

console.info('Calling rule');
rules.runRule('longrunning', {}, true);
console.info('Rule returned');

with longrunning

console.info('Long running rule called');
Java.type('java.lang.Thread').sleep(5000);
console.info('Sleep done');

produces the following output:

2024-01-29 11:06:35.142 [INFO ] [nhab.automation.script.ui.scratchpad] - Calling rule
2024-01-29 11:06:35.978 [INFO ] [hab.automation.script.ui.longrunning] - Long running rule called
2024-01-29 11:06:40.997 [INFO ] [hab.automation.script.ui.longrunning] - Sleep done
2024-01-29 11:06:40.997 [INFO ] [nhab.automation.script.ui.scratchpad] - Rule returned

If I understand this proposal that behavior would change and the call to runRule would immediately return. This could break some end user's rules if they depend on this blocking behavior.

rkoshak avatar Jan 29 '24 18:01 rkoshak

Hello)

It’s not a change, but an addition.

runNow methods of the org.openhab.core.automation.RuleManager are still in place and work the same way as before.

It’s up to automation scripting addons maintainers/designers to decide how to expose new calling methods (instead or besides the existing) and whether to expose them at all.

My aim is to expose some API for running rules to the Groovy scripting without the need to resort to getting services from the OSGi and without errors on parallel execution.

I thought this PR would be a good first step.

OlegAndreych avatar Jan 29 '24 18:01 OlegAndreych

Talking about code snippets.

As I’ve said earlier, it’s up to maintainers/designers of an addon to decide how to expose new API.

One way to solve an issue with immediate return and reordering would be to wait for a Future from one of the newly proposed scheduleRun methods to complete.

Awaiting can be done inside the addon and not be exposed to the addon user/script writer.

Another way can be to return Future/Promise from the runScript (or any new additional method/function to expose new calling method) and let user to decide what to do.

As far as I understand, snippets are JS scripting examples. Unfortunately, I’m not familiar with JS scripting addon internals and not confident that there are no pitfalls with proposed strategies to mitigate early return from scheduleRun.

If you have a proposal on how to make an API more convenient or useful - I’m all ears.

OlegAndreych avatar Jan 29 '24 18:01 OlegAndreych

If this is just additional functionality then I've no concerns. I misunderstood and thought the proposal was to change the current behavior which could cause some problems.

rkoshak avatar Jan 29 '24 19:01 rkoshak

In case there's no more objections, can someone approve pull request?

OlegAndreych avatar Jan 31 '24 14:01 OlegAndreych

I don't have that power. A maintainer needs to come along and review the PR.

rkoshak avatar Jan 31 '24 16:01 rkoshak