diff --git a/code-reviews/Driver.Historian.Wonderware/findings.md b/code-reviews/Driver.Historian.Wonderware/findings.md index cb7b4d4..86c70d1 100644 --- a/code-reviews/Driver.Historian.Wonderware/findings.md +++ b/code-reviews/Driver.Historian.Wonderware/findings.md @@ -7,7 +7,7 @@ | Review date | 2026-05-22 | | Commit reviewed | `76d35d1` | | Status | Reviewed | -| Open findings | 12 | +| Open findings | 11 | ## Checklist coverage @@ -36,7 +36,7 @@ a category produced nothing rather than leaving it blank. | Severity | High | | Category | Correctness and logic bugs | | Location | `Backend/SdkAlarmHistorianWriteBackend.cs:68`, `Backend/AahClientManagedAlarmEventWriter.cs:82-103` | -| Status | Open | +| Status | Resolved | **Description:** `MalformedErrors` includes `HistorianAccessError.ErrorValue.WriteToReadOnlyFile`. When `ClassifyOutcome` routes that code through `MapOutcome`, `isMalformedInput` is @@ -54,7 +54,7 @@ be treated as a connection-class error (abort the batch, reset the connection so the reconnect path can re-open with `ReadOnly = false`) or at minimum as `RetryPlease`, never `PermanentFail`. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-22 — moved `WriteToReadOnlyFile` from `MalformedErrors` into `ConnectionErrors` so the batch loop aborts, resets the connection (re-opening with `ReadOnly = false`), and defers the events as `RetryPlease` instead of dead-lettering them. ### Driver.Historian.Wonderware-002 diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware/Backend/SdkAlarmHistorianWriteBackend.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware/Backend/SdkAlarmHistorianWriteBackend.cs index 60b6184..738a82a 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware/Backend/SdkAlarmHistorianWriteBackend.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware/Backend/SdkAlarmHistorianWriteBackend.cs @@ -56,6 +56,13 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend HistorianAccessError.ErrorValue.Stopping, HistorianAccessError.ErrorValue.Win32Exception, HistorianAccessError.ErrorValue.InvalidResponse, + // WriteToReadOnlyFile is a connection-configuration fault, not an event-payload + // fault: the session was opened without ReadOnly = false (a misconfiguration or + // a regression). The event itself is fine, so it must NOT be dead-lettered. + // Classifying it here aborts the batch and resets the connection so the + // reconnect path re-opens a writable (ReadOnly = false) session; the deferred + // events drain on the next tick. See Driver.Historian.Wonderware-001. + HistorianAccessError.ErrorValue.WriteToReadOnlyFile, }; // ErrorValue codes that mean the event itself is malformed — permanent, never retried. @@ -65,7 +72,6 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend HistorianAccessError.ErrorValue.InvalidArgument, HistorianAccessError.ErrorValue.ValidationFailed, HistorianAccessError.ErrorValue.NullPointerArgument, - HistorianAccessError.ErrorValue.WriteToReadOnlyFile, HistorianAccessError.ErrorValue.NotImplemented, HistorianAccessError.ErrorValue.NotApplicable, }; diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests/Backend/SdkAlarmHistorianWriteBackendTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests/Backend/SdkAlarmHistorianWriteBackendTests.cs index 79b56fa..2be0944 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests/Backend/SdkAlarmHistorianWriteBackendTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests/Backend/SdkAlarmHistorianWriteBackendTests.cs @@ -96,7 +96,6 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Host.Tests [InlineData(HistorianAccessError.ErrorValue.InvalidArgument, AlarmHistorianWriteOutcome.PermanentFail)] [InlineData(HistorianAccessError.ErrorValue.ValidationFailed, AlarmHistorianWriteOutcome.PermanentFail)] [InlineData(HistorianAccessError.ErrorValue.NullPointerArgument, AlarmHistorianWriteOutcome.PermanentFail)] - [InlineData(HistorianAccessError.ErrorValue.WriteToReadOnlyFile, AlarmHistorianWriteOutcome.PermanentFail)] [InlineData(HistorianAccessError.ErrorValue.NotImplemented, AlarmHistorianWriteOutcome.PermanentFail)] public void ClassifyOutcome_maps_error_code_to_expected_outcome( HistorianAccessError.ErrorValue code, AlarmHistorianWriteOutcome expected) @@ -104,6 +103,20 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Host.Tests SdkAlarmHistorianWriteBackend.ClassifyOutcome(code).ShouldBe(expected); } + [Fact] + public void ClassifyOutcome_WriteToReadOnlyFile_is_RetryPlease_not_PermanentFail() + { + // Driver.Historian.Wonderware-001 regression: WriteToReadOnlyFile is a + // connection-configuration fault (the write session was opened without + // ReadOnly = false), NOT a malformed-event fault. Routing it to PermanentFail + // would dead-letter every alarm event in the batch on a misconfigured/regressed + // connection — data loss. It must be treated as a transient connection-class + // error so the events are deferred and retried once the connection is corrected. + SdkAlarmHistorianWriteBackend.ClassifyOutcome( + HistorianAccessError.ErrorValue.WriteToReadOnlyFile) + .ShouldBe(AlarmHistorianWriteOutcome.RetryPlease); + } + // ── BuildConnectionArgs — read-only vs write shaping ────────────────── [Fact]