fix(rule_engine): don't increment unknown counter on unrecoverable er…

[SergeTupchiy]

Closes: EMQX-8786

Fixes

Summary

🤖 Generated by Copilot at 7134acd

Fix rule engine crash bug and refactor is_ok_result function in emqx_rule_runtime.erl

PR Checklist

Please convert it to a draft if any of the following conditions are not met. Reviewers may skip over until all the items are checked:

• Added tests for the changes
• Changed lines covered in coverage report
• Change log has been added to changes/{ce,ee}/(feat|perf|fix)-<PR-id>.en.md files
• For internal contributor: there is a jira ticket to track this change
• If there should be document changes, a PR to emqx-docs.git is sent, or a jira ticket is created to follow up
• Schema changes are backward compatible

Checklist for CI (.github/workflows) changes

• If changed package build workflow, pass this action (manual trigger)
• Change log has been added to changes/ dir for user-facing artifacts update

[SergeTupchiy]

ok is already matched on line 511:

inc_action_metrics(ok, RuleId) ->
    emqx_metrics_worker:inc(rule_metrics, RuleId, 'actions.success');

[zmstone]

if ok results has to be leaked down here anyway we can probably delete line 511 instead.

[SergeTupchiy]

Thanks @zmstone, good point, changed it in: #10328

[SergeTupchiy]

NOTE: this fix is applicable only for synchronous bridges. If a bridge is asynchronous, rule action statistics will count everything as success.

[thalesmg]

Nit: could we add simple test for this case? Could be in a follow up PR to avoid blocking this fix for the release, of course.

Also, maybe we need a changelog here?

[thalesmg]

NOTE: this fix is applicable only for synchronous bridges. If a bridge is asynchronous, rule action statistics will count everything as success.

True, I noticed this issue yesterday as well 🙈
https://emqx.atlassian.net/browse/EMQX-9450

[SergeTupchiy]

Nit: could we add simple test for this case? Could be in a follow up PR to avoid blocking this fix for the release, of course.

Also, maybe we need a changelog here?
Thanks, @thalesmg, I've added a changelog, working on a test case, I think I'll add it to this PR.

[SergeTupchiy]

Nit: could we add simple test for this case? Could be in a follow up PR to avoid blocking this fix for the release, of course.
Also, maybe we need a changelog here?
Thanks, @thalesmg, I've added a changelog, working on a test case, I think I'll add it to this PR.

@thalesmg , I'm going to merge it without tests in order not to block the release, as adding them may take some time.
I've added a follow-up ticket for tests https://emqx.atlassian.net/browse/EMQX-9459