Log Herald rules with invalid actions via phlog()
ClosedPublic
Actions

Authored by aklapper on Jul 22 2024, 11:39.

Details

Reviewers

avivey

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15887: Herald rule referring to a non-existing action silently fails

Commits

rP0bfe9ca88f41: Log Herald rules with invalid actions via phlog()

Summary

When Herald rules fail unexpectedly due to relying on a (now) invalid action, allow Phorge administrators to get aware by creating an entry in the error log via phlog() instead of crossing fingers that someone may from time to time get the idea to open and read Herald transcripts.

Refs T15887

Test Plan

See steps in T15887; check error log.

Diff Detail

Repository

rP Phorge

Branch

heraldRuleInvalidAction

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 1447
Build 1447: arc lint + arc unit

Event Timeline

aklapper created this revision.Jul 22 2024, 11:39

Owners added a reviewer: O1: Blessed Committers.Jul 22 2024, 11:39

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptJul 22 2024, 11:39

aklapper requested review of this revision.Jul 22 2024, 11:39

Harbormaster completed remote builds in B1447: Diff 2259.Jul 22 2024, 11:39

aklapper mentioned this in Z1: Phorge.Aug 1 2024, 12:32

avivey edited the summary of this revision. (Show Details)Aug 2 2024, 06:58

I'm happy to log it, but I'm nos sure this counts as "fixes" the issue - it still requires an admin to monitor the logs.
But then maybe T15887 and its friends can be collected to a single common "more visibility for Herald errors" issue.

This revision is now accepted and ready to land.Aug 2 2024, 07:05

In D25737#20607, @avivey wrote:

I'm happy to log it, but I'm nos sure this counts as "fixes" the issue - it still requires an admin to monitor the logs.
But then maybe T15887 and its friends can be collected to a single common "more visibility for Herald errors" issue.

Yeah, good point. If you have thoughts how to create more visibility for Herald errors which does not require admins to look into logs, please file a ticket referring to https://we.phorge.it/maniphest/?ids=15887,15885,15869#R. Thanks for the review!

Closed by commit rP0bfe9ca88f41: Log Herald rules with invalid actions via phlog(). · Explain WhyAug 4 2024, 09:36

This revision was automatically updated to reflect the committed changes.

aklapper added a commit: rP0bfe9ca88f41: Log Herald rules with invalid actions via phlog().

aklapper mentioned this in Next Up.Aug 12 2024, 09:21

valerio.bozzolan mentioned this in 2024 week 35.Aug 28 2024, 10:19