I have a rule that calls GitHub api (using github_api module) to create branches and fetch code diffs.

This is a required step in my workflow so if, for any reason, I can't "talk" with GitHub I would like to abort the other actions that my rule fires and display an error message to the user.

I've been searching for a while but haven't found a way to stop the regular flow of the rules. Is it even possible to do that?

If it is possible, how can I do it?

CommentFileSizeAuthor
#7 rules-2288427-abort-rules-on-exception-with-tests.patch4.42 KBjribeiro
#2 rules-2288427-abort-rules-on-exception.patch1.45 KBnatanmoraes
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

natanmoraes’s picture

natanmoraes
he/him
Primary language Portuguese, Brazil
CreditAttribution: natanmoraes commented
Category: Support request » Feature request

After searching and debugging Rules's code, I couldn't find a way to stop my rule from executing when an error occurs.

What do you think about adding a "RulesFatalException" exception? If that exception is thrown rules would abort the executing of the rule. I am looking at implementing this in the coming weeks.

natanmoraes’s picture

natanmoraes
he/him
Primary language Portuguese, Brazil
CreditAttribution: natanmoraes commented
Status: Active » Needs review
FileSize
rules-2288427-abort-rules-on-exception.patch1.45 KB

I've updated the module so whenever a rules action callback function throws RulesFatalException, the Rules module will display an error on Rules' log and on Watchdog and will stop any further action from being executed.

gabrielmachadosantos’s picture

gabrielmachadosantos CreditAttribution: gabrielmachadosantos commented
Status: Needs review » Reviewed & tested by the community

+1 RTBC

jribeiro’s picture

jribeiro CreditAttribution: jribeiro commented

+1 RTBC

skwashd’s picture

skwashd CreditAttribution: skwashd commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Based on twitter exchange with @dasjo, this needs test coverage. Updating accordingly.

jribeiro’s picture

jribeiro CreditAttribution: jribeiro commented
Assigned: Unassigned » jribeiro

I will work on this tests, so I'm assigning the ticket for me.

jribeiro’s picture

jribeiro CreditAttribution: jribeiro commented
Version: 7.x-2.6 » 7.x-2.x-dev
Assigned: jribeiro » Unassigned
Status: Needs work » Needs review
FileSize
rules-2288427-abort-rules-on-exception-with-tests.patch4.42 KB

New Patch containing the previous changes plus tests.

Thanks @natanmoraes.

skwashd’s picture

skwashd CreditAttribution: skwashd commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Looks good to me.

skwashd’s picture

skwashd CreditAttribution: skwashd commented

dixon_ queued 7: rules-2288427-abort-rules-on-exception-with-tests.patch for re-testing.

dixon_’s picture

dixon_
he/him
Primary language English
Location France
CreditAttribution: dixon_ commented

Giving the patch another go with the bot, since it was 2 months since it was green...

dixon_’s picture

dixon_
he/him
Primary language English
Location France
CreditAttribution: dixon_ commented

Patch still seems to apply and the patch is also green. +1 RTBC :)

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented
Status: Reviewed & tested by the community » Needs work

Thanks folks, I like the feature. But with the current evaluation it will continue execution if used e.g. inside a loop. Imo it would make most sense to always abort at configuration / component level would make most sense. By exposing an action to throw an exception, you could even do so conditionally from a rule set also.

We should extend test coverage a bit also, such that we ensure it works properly with nested configurations like loops.

  • +++ b/includes/rules.core.inc
@@ -2752,6 +2758,14 @@ class RulesEvaluationException extends RulesException {
+class RulesFatalException extends RulesException {}

    Aborting does not seem to be "fatal", so why not call the eception the same - RulesAbortEvaluationException or so ? 

  • +++ b/tests/rules.test
@@ -930,6 +930,44 @@ class RulesTestCase extends DrupalWebTestCase {
+   * Make sure Rules abort execution when some action gets a fatal error.

    aborts 

  • +++ b/tests/rules.test
@@ -930,6 +930,44 @@ class RulesTestCase extends DrupalWebTestCase {
+  function testRulesFatalError() {

    public

    • jribeiro’s picture

    jribeiro CreditAttribution: jribeiro commented
    Assigned: Unassigned » jribeiro
    jribeiro’s picture

    jribeiro CreditAttribution: jribeiro commented
    Assigned: jribeiro » Unassigned

    When occurs the exception inside a loop, the loop breaks and the consecutive actions outside of the loop are executed.

    We need to define the best approach when get the exception:

    Case 1: Break only the current loop interaction, executing the next loop interaction and the consecutive actions outside of the loop.
    Case 2: Break the entire loop and execute the consecutive actions outside of the loop.
    Case 3: Abort the rule when get the first exception.

    @fago can you explain better your idea about it?

    Thanks

    dasjo’s picture

    dasjo
    he/him
    Primary language German
    Location Zurich
    CreditAttribution: dasjo commented

    my understanding is that fago's idea stated in #13 is case 3: aborting the entire rule component being executed

    johnorourke’s picture

    johnorourke CreditAttribution: johnorourke commented

    Subscribing