fix(driver-historian-wonderware): resolve High code-review finding (Driver.Historian.Wonderware-001)
WriteToReadOnlyFile was listed in MalformedErrors, so ClassifyOutcome/ MapOutcome routed it to PermanentFail and the store-and-forward sink dead-lettered every alarm event in the batch. But WriteToReadOnlyFile is a connection-configuration fault (the write session was opened without ReadOnly = false), not an event-payload fault — treating it as permanent silently and permanently discards alarm events on a misconfigured or regressed connection, which is data loss. Move WriteToReadOnlyFile from MalformedErrors into ConnectionErrors. The batch loop now aborts the batch, resets the connection (so the reconnect path re-opens a writable ReadOnly = false session), and defers the events as RetryPlease for the next drain tick. Updated the ClassifyOutcome theory data and added a dedicated regression test pinning WriteToReadOnlyFile -> RetryPlease. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -7,7 +7,7 @@
|
|||||||
| Review date | 2026-05-22 |
|
| Review date | 2026-05-22 |
|
||||||
| Commit reviewed | `76d35d1` |
|
| Commit reviewed | `76d35d1` |
|
||||||
| Status | Reviewed |
|
| Status | Reviewed |
|
||||||
| Open findings | 12 |
|
| Open findings | 11 |
|
||||||
|
|
||||||
## Checklist coverage
|
## Checklist coverage
|
||||||
|
|
||||||
@@ -36,7 +36,7 @@ a category produced nothing rather than leaving it blank.
|
|||||||
| Severity | High |
|
| Severity | High |
|
||||||
| Category | Correctness and logic bugs |
|
| Category | Correctness and logic bugs |
|
||||||
| Location | `Backend/SdkAlarmHistorianWriteBackend.cs:68`, `Backend/AahClientManagedAlarmEventWriter.cs:82-103` |
|
| Location | `Backend/SdkAlarmHistorianWriteBackend.cs:68`, `Backend/AahClientManagedAlarmEventWriter.cs:82-103` |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
|
|
||||||
**Description:** `MalformedErrors` includes `HistorianAccessError.ErrorValue.WriteToReadOnlyFile`.
|
**Description:** `MalformedErrors` includes `HistorianAccessError.ErrorValue.WriteToReadOnlyFile`.
|
||||||
When `ClassifyOutcome` routes that code through `MapOutcome`, `isMalformedInput` is
|
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
|
the reconnect path can re-open with `ReadOnly = false`) or at minimum as
|
||||||
`RetryPlease`, never `PermanentFail`.
|
`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
|
### Driver.Historian.Wonderware-002
|
||||||
|
|
||||||
|
|||||||
@@ -56,6 +56,13 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
|
|||||||
HistorianAccessError.ErrorValue.Stopping,
|
HistorianAccessError.ErrorValue.Stopping,
|
||||||
HistorianAccessError.ErrorValue.Win32Exception,
|
HistorianAccessError.ErrorValue.Win32Exception,
|
||||||
HistorianAccessError.ErrorValue.InvalidResponse,
|
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.
|
// 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.InvalidArgument,
|
||||||
HistorianAccessError.ErrorValue.ValidationFailed,
|
HistorianAccessError.ErrorValue.ValidationFailed,
|
||||||
HistorianAccessError.ErrorValue.NullPointerArgument,
|
HistorianAccessError.ErrorValue.NullPointerArgument,
|
||||||
HistorianAccessError.ErrorValue.WriteToReadOnlyFile,
|
|
||||||
HistorianAccessError.ErrorValue.NotImplemented,
|
HistorianAccessError.ErrorValue.NotImplemented,
|
||||||
HistorianAccessError.ErrorValue.NotApplicable,
|
HistorianAccessError.ErrorValue.NotApplicable,
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -96,7 +96,6 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Host.Tests
|
|||||||
[InlineData(HistorianAccessError.ErrorValue.InvalidArgument, AlarmHistorianWriteOutcome.PermanentFail)]
|
[InlineData(HistorianAccessError.ErrorValue.InvalidArgument, AlarmHistorianWriteOutcome.PermanentFail)]
|
||||||
[InlineData(HistorianAccessError.ErrorValue.ValidationFailed, AlarmHistorianWriteOutcome.PermanentFail)]
|
[InlineData(HistorianAccessError.ErrorValue.ValidationFailed, AlarmHistorianWriteOutcome.PermanentFail)]
|
||||||
[InlineData(HistorianAccessError.ErrorValue.NullPointerArgument, AlarmHistorianWriteOutcome.PermanentFail)]
|
[InlineData(HistorianAccessError.ErrorValue.NullPointerArgument, AlarmHistorianWriteOutcome.PermanentFail)]
|
||||||
[InlineData(HistorianAccessError.ErrorValue.WriteToReadOnlyFile, AlarmHistorianWriteOutcome.PermanentFail)]
|
|
||||||
[InlineData(HistorianAccessError.ErrorValue.NotImplemented, AlarmHistorianWriteOutcome.PermanentFail)]
|
[InlineData(HistorianAccessError.ErrorValue.NotImplemented, AlarmHistorianWriteOutcome.PermanentFail)]
|
||||||
public void ClassifyOutcome_maps_error_code_to_expected_outcome(
|
public void ClassifyOutcome_maps_error_code_to_expected_outcome(
|
||||||
HistorianAccessError.ErrorValue code, AlarmHistorianWriteOutcome expected)
|
HistorianAccessError.ErrorValue code, AlarmHistorianWriteOutcome expected)
|
||||||
@@ -104,6 +103,20 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Host.Tests
|
|||||||
SdkAlarmHistorianWriteBackend.ClassifyOutcome(code).ShouldBe(expected);
|
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 ──────────────────
|
// ── BuildConnectionArgs — read-only vs write shaping ──────────────────
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
|
|||||||
Reference in New Issue
Block a user