diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs index bdaa1702..b3eae57d 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs @@ -155,6 +155,20 @@ public sealed class SqlDriver private DriverHealth _health = new(DriverState.Unknown, null, null); + /// + /// The exception instance has already turned into a health verdict, held only + /// until — which the poll engine calls with that same object — has + /// consumed it (Driver.Sql-004). + /// + /// + /// A field rather than a flag on the exception itself: is provider-owned + /// and may be read-only, and wrapping the throw would change the type the engine and + /// 's callers see. Concurrent poll loops can overwrite each other's mark; the + /// loser is then merely re-degraded with a poorer message, which is the pre-fix behaviour rather than + /// a new failure. + /// + private Exception? _classifiedByRead; + /// Occurs when a subscribed tag's value or quality changes. public event EventHandler? OnDataChange; @@ -528,6 +542,10 @@ public sealed class SqlDriver // on the log sink via the LogWarning exception parameter above. Degrade($"Sql driver '{_driverInstanceId}' could not reach {Endpoint}: {ex.GetType().Name}."); TransitionHostTo(HostState.Stopped); + // Mark THIS exception object as already classified before it goes back to the poll engine, which + // routes it to HandlePollError next. The mark is what makes the deferral there exact — see that + // method for why inferring it from the exception's type was not (Driver.Sql-004). + Volatile.Write(ref _classifiedByRead, ex); throw; } } @@ -650,11 +668,20 @@ public sealed class SqlDriver /// The exception the poll engine caught. internal void HandlePollError(Exception ex) { - // A subscribed-read outage surfaces as a DbException the reader threw, which ReadAsync has ALREADY - // classified — good, endpoint-bearing LastError + host Stopped — before rethrowing. The poll engine - // then routes that same exception here. Re-degrading would overwrite the good LastError with a worse, - // guidance-free one and log the outage a second time, so skip the connection class ReadAsync owns. - if (ex is DbException) return; + // A subscribed-read outage is classified by ReadAsync — good, endpoint-bearing LastError + host + // Stopped — before it rethrows, and the poll engine then routes that SAME exception object here. + // Re-degrading would overwrite the good LastError with a worse, guidance-free one and log the outage + // a second time, so defer to the classification that already happened. + // + // Matched on the exception INSTANCE ReadAsync marked, not on its type. `ex is DbException` was the + // stated intent — "the class ReadAsync owns" — but not what it implemented (Driver.Sql-004): the + // reader raises a plain InvalidOperationException when the provider factory returns no connection, + // and a provider can raise ArgumentException/FormatException off a malformed connection string on + // the same open path. Those are classified by ReadAsync exactly like a DbException, yet a type test + // let them through and clobbered its message; conversely a DbException that never went through + // ReadAsync — the poll engine's own throw — was swallowed. Consumed once, so a later unclassified + // exception is never mistaken for this one. + if (ReferenceEquals(Interlocked.CompareExchange(ref _classifiedByRead, null, ex), ex)) return; // Whatever reaches here — an engine contract-violation, or a non-DbException the provider raised // (e.g. an ArgumentException from a malformed connection string, which the parser echoes with diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverTests.cs index 9533b8bf..1a52bd2c 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverTests.cs @@ -391,23 +391,91 @@ public sealed class SqlDriverTests { // On a subscribed-read outage the reader throws a DbException; ReadAsync classifies health with a // good, endpoint-bearing message + host Stopped and rethrows the SAME exception, which the poll - // engine then routes to HandlePollError. HandlePollError must ignore the DbException class ReadAsync - // owns — pre-fix it re-degraded with the bare ex.Message (the WORSE of the two LastErrors, and a - // credential-leak path) and logged the outage a second time. + // engine then routes to HandlePollError. HandlePollError must leave the classification ReadAsync + // already made alone — pre-fix it re-degraded with the bare ex.Message (the WORSE of the two + // LastErrors, and a credential-leak path) and logged the outage a second time. + // Driven through the REAL round trip rather than by handing HandlePollError a bare exception: the + // deferral is owned by ReadAsync marking the throw, not by the exception's type (Driver.Sql-004). using var fixture = new SqlitePollFixture(); var logger = new CapturingLogger(); - await using var driver = NewDriver(fixture, logger, KvEntry("Speed", SqlitePollFixture.PresentKey)); + var factory = new BreakableFactory(fixture.Factory); + await using var driver = NewDriver( + fixture, fixture.ConnectionString, logger, factory, KvEntry("Speed", SqlitePollFixture.PresentKey)); await driver.InitializeAsync(ConfigJson, CancellationToken.None); driver.GetHealth().State.ShouldBe(DriverState.Healthy); + factory.BreakByThrowingOnOpen("connection failed; Password=" + SecretToken); + var thrown = await Should.ThrowAsync( + async () => await driver.ReadAsync(["Speed"], CancellationToken.None)); + + var classified = driver.GetHealth(); + classified.State.ShouldBe(DriverState.Degraded); + classified.LastError.ShouldNotBeNull(); + classified.LastError.ShouldContain("could not reach"); + var warningsAfterRead = logger.Entries.Count(e => e.Level == LogLevel.Warning); + + driver.HandlePollError(thrown); + + var health = driver.GetHealth(); + // Untouched: HandlePollError left the exception to ReadAsync, so it neither re-degraded nor leaked… + health.LastError.ShouldBe(classified.LastError); + health.LastError.ShouldNotContain(SecretToken); + // …and it logged nothing (ReadAsync owns the single outage warning). + logger.Entries.Count(e => e.Level == LogLevel.Warning).ShouldBe(warningsAfterRead); + } + + [Fact] + public async Task HandlePollError_forANonDbExceptionReadAsyncClassified_stillDefers_soTheGoodLastErrorSurvives() + { + // Driver.Sql-004. Not every unreachable-database throw is a DbException: the reader raises a plain + // InvalidOperationException when the provider factory hands back no connection, and a provider can + // raise ArgumentException/FormatException off a malformed connection string on the same open path. + // ReadAsync classifies those exactly as it classifies a DbException — endpoint-bearing LastError, + // host Stopped — and rethrows, so HandlePollError must defer to it here too. RED against the + // `ex is DbException` type test, which re-degraded with the guidance-free "poll failed: TypeName" + // and logged the outage twice. + using var fixture = new SqlitePollFixture(); + var logger = new CapturingLogger(); + var factory = new BreakableFactory(fixture.Factory); + await using var driver = NewDriver( + fixture, fixture.ConnectionString, logger, factory, KvEntry("Speed", SqlitePollFixture.PresentKey)); + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + + factory.BreakByReturningNoConnection(); + var thrown = await Should.ThrowAsync( + async () => await driver.ReadAsync(["Speed"], CancellationToken.None)); + + var classified = driver.GetHealth(); + classified.State.ShouldBe(DriverState.Degraded); + classified.LastError.ShouldNotBeNull(); + classified.LastError.ShouldContain("could not reach"); + driver.GetHostStatuses()[0].State.ShouldBe(HostState.Stopped); + var warningsAfterRead = logger.Entries.Count(e => e.Level == LogLevel.Warning); + + driver.HandlePollError(thrown); + + var health = driver.GetHealth(); + health.LastError.ShouldBe(classified.LastError); // the richer message survived… + health.LastError.ShouldNotContain("poll failed"); // …and was not overwritten by the poor one + logger.Entries.Count(e => e.Level == LogLevel.Warning).ShouldBe(warningsAfterRead); + } + + [Fact] + public async Task HandlePollError_forAnExceptionReadAsyncNeverSaw_degrades_evenWhenItIsADbException() + { + // The falsifiability control for the two deferrals above: the marker is per-exception, not per-type, + // so a DbException that never went through ReadAsync's classification is NOT owned by it and must + // still reach the health surface. A type test would silently swallow this one. + using var fixture = new SqlitePollFixture(); + var driver = NewDriver(fixture, KvEntry("Speed", SqlitePollFixture.PresentKey)); + driver.HandlePollError(new FakeDbException("connection failed; Password=" + SecretToken)); var health = driver.GetHealth(); - // Untouched: HandlePollError left the DbException to ReadAsync, so it neither degraded nor leaked… - health.State.ShouldBe(DriverState.Healthy); - health.LastError.ShouldBeNull(); - // …and it logged nothing (ReadAsync owns the single outage warning). - logger.Entries.Count(e => e.Level == LogLevel.Warning).ShouldBe(0); + health.State.ShouldBe(DriverState.Degraded); + health.LastError.ShouldNotBeNull(); + health.LastError.ShouldNotContain(SecretToken); // type only, never the message text + health.LastError.ShouldContain(nameof(FakeDbException)); } [Fact] @@ -518,6 +586,26 @@ public sealed class SqlDriverTests operationTimeout: TimeSpan.FromSeconds(15), commandTimeout: TimeSpan.FromSeconds(10), rawTags: rawTags); + /// The same driver, over a caller-supplied provider factory — the outage-injection seam. + private static SqlDriver NewDriver( + SqlitePollFixture fixture, + string connectionString, + CapturingLogger logger, + DbProviderFactory factory, + params RawTagEntry[] rawTags) + => new( + new SqlDriverOptions + { + RawTags = rawTags, + OperationTimeout = TimeSpan.FromSeconds(15), + CommandTimeout = TimeSpan.FromSeconds(10), + }, + DriverInstanceId, + new SqliteDialect(), + connectionString, + factory: factory, + logger: logger); + /// /// Builds the driver through its production constructor. That constructor is the injection /// seam — dialect, provider factory and already-resolved connection string are all parameters — so no @@ -668,6 +756,28 @@ public sealed class SqlDriverTests public override DbConnection CreateConnection() => new ThrowingConnection(openFailureMessage); } + /// + /// A provider factory that behaves exactly like the fixture's until it is broken, then reproduces one + /// of the two open-path outages the driver has to classify: a factory that returns no + /// connection (a plain from the reader — NOT a + /// ), or one whose connections throw on open. + /// + private sealed class BreakableFactory(DbProviderFactory inner) : DbProviderFactory + { + private Func? _broken; + + /// Hands back no connection, the shape that makes the reader raise InvalidOperationException. + public void BreakByReturningNoConnection() => Volatile.Write(ref _broken, static () => null); + + /// Hands back a connection that fails to open with . + /// The provider message, which may carry credentials. + public void BreakByThrowingOnOpen(string message) + => Volatile.Write(ref _broken, () => new ThrowingConnection(message)); + + public override DbConnection? CreateConnection() + => Volatile.Read(ref _broken) is { } broken ? broken() : inner.CreateConnection(); + } + /// A connection that throws on open; every other member is inert. private sealed class ThrowingConnection(string openFailureMessage) : DbConnection {