fix(alarm-historian): resolve Medium code-review finding (Core.AlarmHistorian-007)
When WriteBatchAsync returned a wrong-cardinality outcome list, DrainOnceAsync threw InvalidOperationException after potential delivery — causing duplicate events on re-drain or permanent queue stall on a deterministic writer bug. - The throw replaced with log + backoff: mismatch is recorded into _lastError, _drainState set to BackingOff, backoff bumped, method returns without applying any outcomes, mirroring the writer-exception path. - Regression test Writer_returning_wrong_cardinality_outcomes_sets_backing_off_and_keeps_rows asserts rows stay queued, DrainState = BackingOff, LastError populated, and that a fixed writer subsequently drains cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -123,13 +123,13 @@
|
||||
| Severity | Medium |
|
||||
| Category | Error handling & resilience |
|
||||
| Location | `src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs:172-174` |
|
||||
| Status | Open |
|
||||
| Status | Resolved |
|
||||
|
||||
**Description:** When the writer returns a wrong-cardinality result, the code throws `InvalidOperationException` after `WriteBatchAsync` has already succeeded. The events were potentially delivered to the historian, but no rows are deleted or dead-lettered, `_drainState` is left at `Draining`, and the backoff is not bumped. Combined with Core.AlarmHistorian-006 the exception is then swallowed. On the next drain the same batch is re-sent — if the writer actually delivered the events the first time, this produces duplicate historian rows; if it is a deterministic writer bug the queue stalls forever.
|
||||
|
||||
**Recommendation:** Treat a cardinality mismatch as a transient batch failure: log it, set `_lastError`, bump backoff, set `_drainState = BackingOff`, and return without throwing — mirroring the writer-exception path at lines 162-170. A deterministic writer contract violation should also raise an operator-visible alert rather than silently looping.
|
||||
|
||||
**Resolution:** _(open)_
|
||||
**Resolution:** Resolved 2026-05-22 — the `throw InvalidOperationException` replaced with log-and-backoff: mismatch is recorded into `_lastError`, `_drainState = BackingOff`, backoff is bumped, and the method returns without applying any outcomes — rows stay queued for the next drain attempt. Regression test `Writer_returning_wrong_cardinality_outcomes_sets_backing_off_and_keeps_rows` added.
|
||||
|
||||
### Core.AlarmHistorian-008
|
||||
|
||||
|
||||
Reference in New Issue
Block a user