fix(twincat): per-device connect-attempt backoff, probe-driven recovery (05/STAB-8)

archreview/plans/R2-09-driver-fleet-batch-plan.md.tasks.json

@@ -121,7 +121,7 @@
     {
       "id": "B4.1",
       "subject": "B4: TwinCAT per-device connect-attempt backoff (data-path gate, probe bypass + instant reset) tests FAIL->PASS \u2014 STAB-8",
-      "status": "pending",
+      "status": "completed",
       "blockedBy": [
         "B3.2"
       ]

src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs

@@ -647,7 +647,9 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
             {
                 // Probe-initiated connects honor TwinCATProbeOptions.Timeout — distinct from
                 // the driver-wide _options.Timeout used by reads/writes.
-                var client = await EnsureConnectedAsync(state, ct, _options.Probe.Timeout)
+                // The probe bypasses the connect backoff — its own interval is the bounded recovery
+                // cadence, and a successful probe connect resets the window for the data path.
+                var client = await EnsureConnectedAsync(state, ct, _options.Probe.Timeout, bypassBackoff: true)
                     .ConfigureAwait(false);
                 success = await client.ProbeAsync(ct).ConfigureAwait(false);
 
@@ -723,7 +725,7 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
     ///     also what docs/v2/driver-specs.md recommends.
     /// </summary>
     private async Task<ITwinCATClient> EnsureConnectedAsync(
-        DeviceState device, CancellationToken ct, TimeSpan? timeoutOverride = null)
+        DeviceState device, CancellationToken ct, TimeSpan? timeoutOverride = null, bool bypassBackoff = false)
     {
         // Fast path — already connected, no gate needed.
         if (device.Client is { IsConnected: true } fast) return fast;
@@ -731,7 +733,7 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
         await device.ConnectGate.WaitAsync(ct).ConfigureAwait(false);
         try
         {
-            return await EnsureConnectedUnderGateAsync(device, ct, timeoutOverride).ConfigureAwait(false);
+            return await EnsureConnectedUnderGateAsync(device, ct, timeoutOverride, bypassBackoff).ConfigureAwait(false);
         }
         finally
         {
@@ -739,6 +741,13 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
         }
     }
 
+    /// <summary>
+    ///     Thrown by the connect path when a per-device <see cref="ConnectionBackoff"/> window is
+    ///     still open (05/STAB-8) — a fail-fast that costs no wire traffic. Read/write map it to
+    ///     <c>BadCommunicationError</c> via their generic catch, exactly as a real connect failure.
+    /// </summary>
+    private sealed class ConnectionThrottledException(string message) : Exception(message);
+
     /// <summary>
     ///     The connect-or-reconnect core, assuming the caller already holds
     ///     <see cref="DeviceState.ConnectGate"/>. When it installs a NEW client (a genuine
@@ -748,11 +757,18 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
     ///     through here under the same gate, so a replay can never race a register.
     /// </summary>
     private async Task<ITwinCATClient> EnsureConnectedUnderGateAsync(
-        DeviceState device, CancellationToken ct, TimeSpan? timeoutOverride)
+        DeviceState device, CancellationToken ct, TimeSpan? timeoutOverride, bool bypassBackoff = false)
     {
         // Re-check under the gate: another caller may have connected while we waited.
         if (device.Client is { IsConnected: true } c) return c;
 
+        // 05/STAB-8: inside an open backoff window a data-path caller fails fast — no client
+        // create, no wire connect. The probe loop bypasses this (its interval is the recovery
+        // cadence) and drives the failure/success recording.
+        if (!bypassBackoff && !device.Backoff.ShouldAttempt(DateTime.UtcNow))
+            throw new ConnectionThrottledException(
+                $"TwinCAT connect to {device.Options.HostAddress} throttled — backoff window open after a recent connect failure.");
+
         // Discard a stale (created-but-disconnected) client before making a fresh one.
         if (device.Client is { IsConnected: false } stale)
         {
@@ -782,9 +798,11 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
                 _driverInstanceId, device.Options.HostAddress);
             client.OnSymbolVersionChanged -= HandleSymbolVersionChanged;
             client.Dispose();
+            device.Backoff.RecordFailure(DateTime.UtcNow); // 05/STAB-8: open the backoff window
             throw;
         }
         device.Client = client;
+        device.Backoff.RecordSuccess(); // 05/STAB-8: recovery resets the window immediately
 
         // Re-register every native notification onto the fresh client. Native ADS is push, so
         // (unlike the poll fallback) it cannot self-heal by re-reading — without this the
@@ -930,6 +948,15 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
         ///     create-or-dispose for this device.</summary>
         public SemaphoreSlim ConnectGate { get; } = new(1, 1);
 
+        /// <summary>
+        ///     Per-device connect-attempt throttle (05/STAB-8). A dead device fails fast inside the
+        ///     backoff window instead of paying a full connect on every data call; the probe loop
+        ///     bypasses it and a successful connect resets it. Consulted + mutated only under
+        ///     <see cref="ConnectGate"/>, satisfying <see cref="ConnectionBackoff"/>'s
+        ///     caller-synchronized contract.
+        /// </summary>
+        public ConnectionBackoff Backoff { get; } = new(TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(30));
+
         /// <summary>
         ///     Live native-notification registrations against this device, keyed by registration id.
         ///     Holds the <b>replayable intent</b> (symbol / type / bit / interval / callback), not just

tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATConnectBackoffTests.cs

@@ -0,0 +1,90 @@
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
+using ZB.MOM.WW.OtOpcUa.Driver.TwinCAT;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests;
+
+/// <summary>
+///     05/STAB-8 — a dead TwinCAT device must not be hammered with a full connect attempt on every
+///     data call. The per-device <see cref="ConnectionBackoff"/> gates the connect/create path;
+///     the probe loop bypasses it (its own interval is the recovery cadence) and a successful
+///     connect resets the window instantly so recovery is never delayed. No TC3 fixture exists —
+///     the fake-client suite is authoritative.
+/// </summary>
+[Trait("Category", "Unit")]
+public sealed class TwinCATConnectBackoffTests
+{
+    private const string Host = "ads://5.23.91.23.1.1:851";
+
+    private static TwinCATDriverOptions Options(bool probeEnabled) => new()
+    {
+        Devices = [new TwinCATDeviceOptions(Host)],
+        Tags = [new TwinCATTagDefinition("T", Host, "MAIN.T", TwinCATDataType.DInt)],
+        Probe = new TwinCATProbeOptions
+        {
+            Enabled = probeEnabled,
+            Interval = TimeSpan.FromMilliseconds(50),
+            Timeout = TimeSpan.FromMilliseconds(250),
+        },
+        EnableControllerBrowse = false,
+    };
+
+    /// <summary>Rapid data calls against a dead device attempt a single connect, then fail fast inside the backoff window.</summary>
+    [Fact]
+    public async Task DeadDevice_ConnectAttemptsFollowBackoffSchedule()
+    {
+        var factory = new FakeTwinCATClientFactory
+        {
+            Customise = () => new FakeTwinCATClient
+            {
+                ThrowOnConnect = true,
+                Exception = new InvalidOperationException("device down"),
+            },
+        };
+        var drv = new TwinCATDriver(Options(probeEnabled: false), "twincat-1", factory);
+        await drv.InitializeAsync("{}", CancellationToken.None);
+
+        for (var i = 0; i < 6; i++)
+            await drv.ReadAsync(["T"], CancellationToken.None);
+
+        // One real connect attempt; the remaining five fail fast inside the 1 s window — no new
+        // client is created (the fake factory records one client per attempted connect).
+        factory.Clients.Count.ShouldBe(1);
+    }
+
+    /// <summary>The probe bypasses the backoff window and a successful connect resets it so the next data call succeeds immediately.</summary>
+    [Fact]
+    public async Task ProbeBypassesBackoff_AndSuccessResets()
+    {
+        var fail = true;
+        var factory = new FakeTwinCATClientFactory
+        {
+            Customise = () => new FakeTwinCATClient
+            {
+                ThrowOnConnect = fail,
+                Exception = new InvalidOperationException("device down"),
+            },
+        };
+        var drv = new TwinCATDriver(Options(probeEnabled: true), "twincat-1", factory);
+        await drv.InitializeAsync("{}", CancellationToken.None);
+
+        // First data call opens the backoff window on a dead device.
+        var down = await drv.ReadAsync(["T"], CancellationToken.None);
+        down[0].StatusCode.ShouldBe(TwinCATStatusMapper.BadCommunicationError);
+
+        // Device recovers. The data path is still inside the window, but the probe bypasses it and
+        // reconnects — resetting the window so the following data read succeeds with no residual delay.
+        fail = false;
+        var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(3);
+        DataValueSnapshot? good = null;
+        while (DateTime.UtcNow < deadline)
+        {
+            var r = await drv.ReadAsync(["T"], CancellationToken.None);
+            if (r[0].StatusCode == TwinCATStatusMapper.Good) { good = r[0]; break; }
+            await Task.Delay(25, CancellationToken.None);
+        }
+
+        good.ShouldNotBeNull("probe-driven reconnect must reset the backoff so data reads recover");
+    }
+}