openhab / openhab-core

Core framework of openHAB
https://www.openhab.org/
Eclipse Public License 2.0
925 stars 425 forks source link

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

Open OlegAndreych opened 9 months ago

OlegAndreych commented 9 months ago

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.

rkoshak commented 9 months ago

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.

OlegAndreych commented 9 months ago

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 commented 9 months ago

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.

rkoshak commented 9 months ago

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.

OlegAndreych commented 9 months ago

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

rkoshak commented 9 months ago

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

alaub81 commented 2 days ago

I came across this issue, because I also ran into these errors. I created a script to reuse it in 4 rules, which can be triggered at the same time, which is currently not working.

So can someone tell me what will be happening with this pull request? Or is there a workaround for the problem?

rkoshak commented 1 day ago

Or is there a workaround for the problem?

Use a personal library instead of a Script if that is supported for the language you are using.

OlegAndreych commented 21 hours ago

Or is there a workaround for the problem?

You can try this solution https://www.openhab.org/addons/automation/groovyscripting/#code-reuse.

It works for me since I've added it to the doc.

alaub81 commented 5 hours ago

Thanks, will have a look at both solutions.