Problem/Motivation

This is an idea for error message improvement for site admins and developers.

When a Rules reaction rule failed to get a parameter, a error message like the following one is logged with watchdog.

Unable to get variable user, it is not defined.
Unable to get variable node, it is not defined.

This is helpful to detect the problematic reaction rule when a site has a small set of reaction rules. But if a site has a large number of reaction rules, for example, hundreds, the message is not very informative and debugging is hard. If more information is provided in the message, admins and developers can easily detect problematic reaction rules.

(I'd like to propose this though I know of the existing debug function of Rules module. The existing function is useful but sometimes generates too many log messages and overwhelming.)

Proposed resolution

Add the plugin name which raised the exception (which leads to the error log) into the error message.

The image below is a sample capture after applying the patch.

sample capture after applying the patch

Remaining tasks

  • Create a patch
  • Review the patch

User interface changes

(None)

API changes

(None)

Data model changes

(None)

CommentFileSizeAuthor
#4 interdiff-2875575-2-4.txt678 byteshgoto
#4 rules-make_error_message_more_informative-2875575-4.patch4.75 KBhgoto
#2 2875575-2.png25.69 KBhgoto
#2 rules-make_error_message_more_informative-2875575-2.patch4.13 KBhgoto
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hgoto created an issue. See original summary.

hgoto’s picture

hgoto CreditAttribution: hgoto at Studio Umi commented
Status: Active » Needs review
FileSize
rules-make_error_message_more_informative-2875575-2.patch4.13 KB
2875575-2.png25.69 KB

I made a first trial patch.

Here are some points.

  • The error message "Unable to get variable %name, it is not defined." is generated in a method of RulesState. Instances of RulesState don't have any label for human and I'd like to use the parent plugin label instead. For that, we need to add a reference from RulesState to RulesPlugin.
  • The following change into test/rules.test is included in this patch. This change is actually not necessary but I'd like to fix this as the constructor of RulesState originally doesn't take any arguments.

-    $state = new RulesState(rule());
+    $state = new RulesState();

I'd like this to be reviewed.

The attached image is a sample capture after applying the patch.

sample capture

Status: Needs review » Needs work

The last submitted patch, 2: rules-make_error_message_more_informative-2875575-2.patch, failed testing.

hgoto’s picture

hgoto CreditAttribution: hgoto at Studio Umi commented
Status: Needs work » Needs review
FileSize
rules-make_error_message_more_informative-2875575-4.patch4.75 KB
interdiff-2875575-2-4.txt678 bytes

Oops. There was a test which validates the error message. I adjusted the test.

hgoto’s picture

hgoto CreditAttribution: hgoto at Studio Umi commented
Issue summary: View changes
TR’s picture

TR CreditAttribution: TR commented

I agree, this would be a big usability improvement.