diff --git a/archreview/plans/R2-09-driver-fleet-batch-plan.md.tasks.json b/archreview/plans/R2-09-driver-fleet-batch-plan.md.tasks.json index 76641699..40809277 100644 --- a/archreview/plans/R2-09-driver-fleet-batch-plan.md.tasks.json +++ b/archreview/plans/R2-09-driver-fleet-batch-plan.md.tasks.json @@ -12,7 +12,7 @@ { "id": "B1.2", "subject": "B1: ModbusTransportDesyncException + teardown-before-propagate in SendOnceAsync (timeout-OCE + InvalidDataException) \u2014 STAB-3", - "status": "pending", + "status": "completed", "blockedBy": [ "B1.1" ] diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusTcpTransport.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusTcpTransport.cs index 11268eb8..e7701296 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusTcpTransport.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusTcpTransport.cs @@ -207,6 +207,27 @@ public sealed class ModbusTcpTransport : IModbusTransport } } + /// + /// Executes exactly one Modbus transaction on the current socket. + /// + /// + /// + /// A per-op deadline (linked ) + /// and framing violations (TxId mismatch / truncated MBAP length) are treated as + /// connection-fatal: the socket may have a half-read or still-in-flight response, + /// so reusing it would read stale bytes and desynchronize the stream permanently. Such + /// failures tear the socket down here (before propagating) and surface as + /// — which, being an + /// , feeds 's existing single reconnect + /// retry with no extra plumbing. + /// + /// + /// The timeout-vs-caller-cancel distinction is load-bearing: a caller cancellation + /// (ct.IsCancellationRequested) is a legitimate shutdown and propagates as a plain + /// with no teardown — only the linked + /// per-op timeout (the caller token is NOT cancelled) is a desync. + /// + /// private async Task SendOnceAsync(byte unitId, byte[] pdu, CancellationToken ct) { if (_stream is null) throw new InvalidOperationException("Transport not connected"); @@ -225,28 +246,53 @@ public sealed class ModbusTcpTransport : IModbusTransport using var cts = CancellationTokenSource.CreateLinkedTokenSource(ct); cts.CancelAfter(_timeout); - await _stream.WriteAsync(adu.AsMemory(), cts.Token).ConfigureAwait(false); - await _stream.FlushAsync(cts.Token).ConfigureAwait(false); - - var header = new byte[7]; - await ReadExactlyAsync(_stream, header, cts.Token).ConfigureAwait(false); - var respTxId = (ushort)((header[0] << 8) | header[1]); - if (respTxId != txId) - throw new InvalidDataException($"Modbus TxId mismatch: expected {txId} got {respTxId}"); - var respLen = (ushort)((header[4] << 8) | header[5]); - if (respLen < 1) throw new InvalidDataException($"Modbus response length too small: {respLen}"); - var respPdu = new byte[respLen - 1]; - await ReadExactlyAsync(_stream, respPdu, cts.Token).ConfigureAwait(false); - - // Exception PDU: function code has high bit set. - if ((respPdu[0] & 0x80) != 0) + try { - var fc = (byte)(respPdu[0] & 0x7F); - var ex = respPdu[1]; - throw new ModbusException(fc, ex, $"Modbus exception fc={fc} code={ex}"); - } + await _stream.WriteAsync(adu.AsMemory(), cts.Token).ConfigureAwait(false); + await _stream.FlushAsync(cts.Token).ConfigureAwait(false); - return respPdu; + var header = new byte[7]; + await ReadExactlyAsync(_stream, header, cts.Token).ConfigureAwait(false); + var respTxId = (ushort)((header[0] << 8) | header[1]); + if (respTxId != txId) + throw new ModbusTransportDesyncException( + ModbusTransportDesyncException.DesyncReason.TxIdMismatch, + $"Modbus TxId mismatch: expected {txId} got {respTxId}"); + var respLen = (ushort)((header[4] << 8) | header[5]); + if (respLen < 1) + throw new ModbusTransportDesyncException( + ModbusTransportDesyncException.DesyncReason.TruncatedFrame, + $"Modbus response length too small: {respLen}"); + var respPdu = new byte[respLen - 1]; + await ReadExactlyAsync(_stream, respPdu, cts.Token).ConfigureAwait(false); + + // Exception PDU: function code has high bit set. This is a well-formed protocol-level + // error — the socket is still coherent, so it MUST propagate without teardown. + if ((respPdu[0] & 0x80) != 0) + { + var fc = (byte)(respPdu[0] & 0x7F); + var ex = respPdu[1]; + throw new ModbusException(fc, ex, $"Modbus exception fc={fc} code={ex}"); + } + + return respPdu; + } + catch (ModbusTransportDesyncException) + { + // Framing violation: the socket is desynchronized — never reuse it. + await TearDownAsync().ConfigureAwait(false); + throw; + } + catch (OperationCanceledException) when (!ct.IsCancellationRequested) + { + // Per-op timeout fired (NOT the caller). The response is unknown / may still land later + // and corrupt the next read — tear the socket down and normalize as a desync so the + // reconnect retry / status mapping engages. + await TearDownAsync().ConfigureAwait(false); + throw new ModbusTransportDesyncException( + ModbusTransportDesyncException.DesyncReason.Timeout, + $"Modbus per-op response timeout after {_timeout.TotalMilliseconds:0}ms"); + } } /// diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusTransportDesyncException.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusTransportDesyncException.cs new file mode 100644 index 00000000..6f75e270 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusTransportDesyncException.cs @@ -0,0 +1,46 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus; + +/// +/// Raised when a Modbus TCP transaction leaves the single-flight socket in an unknown / +/// desynchronized state — a per-op response timeout, or a framing violation (transaction-id +/// mismatch or truncated MBAP length). Unlike a Modbus exception PDU (a well-formed +/// protocol-level error where the socket is still coherent and the request must simply +/// propagate), a desync means bytes may still be in flight or half-read, so the socket is +/// unusable and MUST be torn down before this is thrown. +/// +/// +/// Derives from deliberately: the transport's existing +/// reconnect-and-retry classifier (IsSocketLevelFailure) already treats +/// as socket-fatal, so with auto-reconnect enabled the transport +/// transparently reconnects and resends once; with auto-reconnect disabled the (already torn +/// down) socket is never reused and this surfaces to the driver's status mapping as a +/// communication error. +/// +public sealed class ModbusTransportDesyncException : IOException +{ + /// The reason the socket was classified desynchronized. + public enum DesyncReason + { + /// The per-op deadline (linked CancelAfter) fired — the response never arrived in time. + Timeout, + + /// The response MBAP transaction id did not match the request's. + TxIdMismatch, + + /// The response MBAP length field was below the mandatory minimum (truncated frame). + TruncatedFrame, + } + + /// Gets the reason the socket was classified desynchronized. + public DesyncReason Reason { get; } + + /// Initializes a new instance of the class. + /// The desync reason. + /// A human-readable description. + /// The originating exception, if any. + public ModbusTransportDesyncException(DesyncReason reason, string message, Exception? inner = null) + : base(message, inner) + { + Reason = reason; + } +}