Page MenuHomePhorge
Create Task
Maniphest T15869

Herald rule referring to a disabled rule silently fails
Open, Needs TriagePublic
Actions

Description

Assume this rule:

H15
when:
    <something happens> 
    AND rule H10 doesn't match,
then:
    send me an email

If rule H10 is disabled, then rule H15 will fail to evaluate, never actually do anything, and there's no way to find that except by visiting a Transcript after you know something is wrong.

In my use-case (H7 -> H22), I expected it to interpret "disable rule" as "doesn't match", but I suspect there are other possible interpretations, depending on context.

In any case, having the referring rule silently fail is bad.

Revisions and Commits

rP Phorge
D25736rP71615f4b9917 Log Herald rules having disabled Herald rules as condition via phlog()

Related Objects
Search...

Event Timeline

avivey created this task.Jun 27 2024, 10:26
Herald added subscribers: Cigaryno, Matthew, chris, tobiaswiese. · View Herald TranscriptJun 27 2024, 10:26
aklapper subscribed.Jun 27 2024, 12:19
aklapper mentioned this in T15885: Herald rule referring to an invalid key silently fails.Jul 19 2024, 13:23
aklapper added a comment.Jul 19 2024, 16:05

Uhmm, transcripts at http://phorge.localhost/herald/transcript/33065/ never ever render a Herald rule condition (disabled or not) within another rule, it seems

aklapper added a comment.Jul 21 2024, 17:17

Ahh but Transcripts do render an exception when it fails at least. I get a HeraldInvalidConditionException printing Invalid Condition: Condition references a rule which does not exist!, triggered in HeraldAdapter::doesConditionMatch() (only if you reach that condition in the evaluation of course, because https://we.phorge.it/source/phorge/browse/master/src/applications/herald/engine/HeraldEngine.php$437-445 ).
That's called in HeraldEngine in the line $is_match = $adapter->doesConditionMatch (note that class also has a method with the very same name).

So I would take the same approach in T15885 / D25735 and phlog() the exception - that'll allow admins to find a hint when checking the error log.

Another option could be checking whether a Herald rule is referenced in a condition or an action whenever disabling that very Herald rule but I think I lack skills to write such code.

aklapper added a revision: D25736: Log Herald rules having disabled Herald rules as condition via phlog().Jul 21 2024, 17:21
aklapper mentioned this in T15887: Herald rule referring to a non-existing action silently fails.Jul 22 2024, 11:36
aklapper added a commit: rP71615f4b9917: Log Herald rules having disabled Herald rules as condition via phlog().Aug 4 2024, 09:35
aklapper mentioned this in Next Up.Mon, Aug 12, 09:21
valerio.bozzolan added a parent task: T15921: Easy way to find broken Herald Rule(s).Mon, Aug 19, 13:01
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0