fix(transport): name serial connect failures instead of calling them all access denials (closes #424)

src/Daqifi.Core.Tests/Communication/Transport/SerialPortConnectExceptionTests.cs

@@ -0,0 +1,191 @@
+using Daqifi.Core.Communication.Transport;
+
+namespace Daqifi.Core.Tests.Communication.Transport;
+
+/// <summary>
+/// Pins the connect-failure translation added for #424: <see cref="SerialStreamTransport"/> no
+/// longer forwards the platform's exception for a failed open, because that exception cannot be
+/// classified. Measured on macOS with System.IO.Ports 10.0.10, a missing port and a port held by
+/// another process both produce
+/// <c>UnauthorizedAccessException("Access to the port 'X' is denied.")</c> wrapping an
+/// <c>IOException("Unknown error: 203")</c> — identical type, text, and HResult — so the reason has
+/// to come from evidence gathered around the failure instead.
+/// </summary>
+public class SerialPortConnectExceptionTests
+{
+    /// <summary>
+    /// The exact exception shape a failed open produces on macOS/Linux, for every cause.
+    /// </summary>
+    private static UnauthorizedAccessException PlatformDenied(string portName = "/dev/cu.fake") =>
+        new($"Access to the port '{portName}' is denied.", new IOException("Unknown error: 203"));
+
+    [Fact]
+    public void Classify_WhenPortIsAbsent_ReportsNotFound()
+    {
+        // The reported bug: a port that does not exist was called an access denial.
+        var reason = SerialPortConnectException.Classify(PlatformDenied(), portPresent: false,
+            permissionGateRuledOut: null);
+
+        Assert.Equal(SerialPortConnectFailure.NotFound, reason);
+    }
+
+    [Fact]
+    public void Classify_WhenPortIsAbsent_IgnoresThePermissionGate()
+    {
+        // Absence is conclusive; a device node that has already vanished cannot be a permission
+        // or an exclusivity problem, whatever the gate probe managed to say.
+        Assert.Equal(SerialPortConnectFailure.NotFound,
+            SerialPortConnectException.Classify(PlatformDenied(), false, permissionGateRuledOut: true));
+        Assert.Equal(SerialPortConnectFailure.NotFound,
+            SerialPortConnectException.Classify(PlatformDenied(), false, permissionGateRuledOut: false));
+    }
+
+    [Fact]
+    public void Classify_FileNotFoundException_ReportsNotFoundWithoutCorroboration()
+    {
+        // Windows names this case outright, so it needs no presence probe to agree with it.
+        var reason = SerialPortConnectException.Classify(
+            new FileNotFoundException("The port 'COM254' does not exist."),
+            portPresent: true,
+            permissionGateRuledOut: true);
+
+        Assert.Equal(SerialPortConnectFailure.NotFound, reason);
+    }
+
+    [Fact]
+    public void Classify_FileNotFoundExceptionNestedInside_ReportsNotFound()
+    {
+        var reason = SerialPortConnectException.Classify(
+            new UnauthorizedAccessException("wrapped", new FileNotFoundException("no such port")),
+            portPresent: true,
+            permissionGateRuledOut: true);
+
+        Assert.Equal(SerialPortConnectFailure.NotFound, reason);
+    }
+
+    [Fact]
+    public void Classify_PresentAndDeniedWhereNoPermissionGateCanApply_ReportsInUse()
+    {
+        // A macOS /dev/cu.* node is crw-rw-rw-, so nobody can be denied on permission grounds:
+        // the port exists and refuses to open because another process holds it.
+        var reason = SerialPortConnectException.Classify(PlatformDenied(), portPresent: true,
+            permissionGateRuledOut: true);
+
+        Assert.Equal(SerialPortConnectFailure.InUse, reason);
+    }
+
+    [Fact]
+    public void Classify_PresentAndDeniedWhereAPermissionGateCouldApply_ReportsAccessDenied()
+    {
+        // A Linux dialout-owned node at crw-rw---- is the genuine permission case.
+        var reason = SerialPortConnectException.Classify(PlatformDenied("/dev/ttyUSB0"),
+            portPresent: true, permissionGateRuledOut: false);
+
+        Assert.Equal(SerialPortConnectFailure.AccessDenied, reason);
+    }
+
+    [Fact]
+    public void Classify_WhenTheGateCannotBeDetermined_DegradesToAccessDenied()
+    {
+        // An unfamiliar platform keeps today's wording rather than asserting something false.
+        var reason = SerialPortConnectException.Classify(PlatformDenied(), portPresent: true,
+            permissionGateRuledOut: null);
+
+        Assert.Equal(SerialPortConnectFailure.AccessDenied, reason);
+    }
+
+    [Fact]
+    public void Classify_WhenPresenceCannotBeObserved_DoesNotInventAbsence()
+    {
+        // A probe that could not answer is not evidence the port is gone. Reporting NotFound here
+        // would turn any unrelated connect failure into a bogus "port was not found".
+        var reason = SerialPortConnectException.Classify(PlatformDenied(), portPresent: null,
+            permissionGateRuledOut: false);
+
+        Assert.Equal(SerialPortConnectFailure.AccessDenied, reason);
+    }
+
+    [Fact]
+    public void Classify_AnUnrecognizedIoFailure_StaysUnknown()
+    {
+        var reason = SerialPortConnectException.Classify(new IOException("the port hardware failed"),
+            portPresent: true, permissionGateRuledOut: true);
+
+        Assert.Equal(SerialPortConnectFailure.Unknown, reason);
+    }
+
+    [Fact]
+    public void Classify_NullError_Throws()
+    {
+        Assert.Throws<ArgumentNullException>(() =>
+            SerialPortConnectException.Classify(null!, true, true));
+    }
+
+    [Theory]
+    [InlineData(SerialPortConnectFailure.NotFound, "was not found")]
+    [InlineData(SerialPortConnectFailure.InUse, "is in use")]
+    [InlineData(SerialPortConnectFailure.AccessDenied, "is denied")]
+    [InlineData(SerialPortConnectFailure.Unknown, "could not be opened")]
+    public void DescribeFailure_NamesThePortAndTheCause(SerialPortConnectFailure reason, string expected)
+    {
+        var message = SerialPortConnectException.DescribeFailure("/dev/cu.usbmodem1101", reason);
+
+        Assert.Contains("/dev/cu.usbmodem1101", message);
+        Assert.Contains(expected, message, StringComparison.OrdinalIgnoreCase);
+    }
+
+    [Fact]
+    public void DescribeFailure_AccessDenied_KeepsTheFrameworkWording()
+    {
+        // The one case the platform always got right; anything already keying on that phrasing for
+        // a real permission problem keeps matching.
+        Assert.Equal("Access to the port '/dev/ttyUSB0' is denied.",
+            SerialPortConnectException.DescribeFailure("/dev/ttyUSB0", SerialPortConnectFailure.AccessDenied));
+    }
+
+    [Fact]
+    public void FromOpenFailure_PreservesTheOriginalAsInnerException()
+    {
+        var original = PlatformDenied();
+
+        var ex = SerialPortConnectException.FromOpenFailure("/dev/cu.fake", original,
+            portPresent: false, permissionGateRuledOut: null);
+
+        Assert.Same(original, ex.InnerException);
+        Assert.Equal("/dev/cu.fake", ex.PortName);
+        Assert.Equal(SerialPortConnectFailure.NotFound, ex.Reason);
+        Assert.Contains("was not found", ex.Message);
+    }
+
+    [Theory]
+    [InlineData(false, null, SerialPortConnectFailure.NotFound)]
+    [InlineData(true, true, SerialPortConnectFailure.InUse)]
+    [InlineData(true, false, SerialPortConnectFailure.AccessDenied)]
+    public void FromOpenFailure_KeepsTheInnerExceptionForEveryReason(
+        bool present, bool? gateRuledOut, SerialPortConnectFailure expected)
+    {
+        var original = PlatformDenied();
+
+        var ex = SerialPortConnectException.FromOpenFailure("/dev/cu.fake", original, present, gateRuledOut);
+
+        Assert.Equal(expected, ex.Reason);
+        Assert.Same(original, ex.InnerException);
+    }
+
+    [Fact]
+    public void SerialPortConnectException_IsAnIoException()
+    {
+        // Failing to open an I/O device is an I/O error, and Windows already reports a missing port
+        // as an IOException-derived type, so catch (IOException) around a connect keeps working.
+        var ex = new SerialPortConnectException("COM3", SerialPortConnectFailure.NotFound, "nope");
+
+        Assert.IsAssignableFrom<IOException>(ex);
+    }
+
+    [Fact]
+    public void SerialPortConnectException_RequiresAPortName()
+    {
+        Assert.Throws<ArgumentNullException>(() =>
+            new SerialPortConnectException(null!, SerialPortConnectFailure.NotFound, "nope"));
+    }
+}

src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs

@@ -141,6 +141,182 @@ public async Task SerialStreamTransport_ConnectAsync_WithInvalidPort_ShouldThrow
         Assert.False(transport.IsConnected);
     }
 
+    /// <summary>
+    /// Produces a port name checked to be absent on the running host, so the missing-port tests
+    /// assert the <see cref="SerialPortConnectFailure.NotFound"/> translation and nothing else.
+    /// </summary>
+    /// <remarks>
+    /// Verified at runtime rather than hard-coded: a fixed name is only absent by assumption, and a
+    /// host that happens to have it — a virtual COM port, a leftover device node — would make these
+    /// tests either open real hardware or fail with a different error shape.
+    /// </remarks>
+    private static string CreateVerifiedAbsentPort()
+    {
+        // Whether the enumeration *answered* is tracked separately from what it returned, because
+        // the two failure modes are not equivalent and must not be collapsed. An empty result is a
+        // real answer — the host has no serial ports. A throw is no answer at all.
+        bool enumerationAnswered;
+        HashSet<string> enumerated;
+        try
+        {
+            enumerated = new HashSet<string>(SerialPort.GetPortNames(), StringComparer.OrdinalIgnoreCase);
+            enumerationAnswered = true;
+        }
+        catch (Exception)
+        {
+            // Enumeration can throw on some hosts (a container without /dev access, a locked-down
+            // machine). It must not take the suite down from inside a test helper — that reads as
+            // the feature under test breaking rather than the environment.
+            enumerated = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
+            enumerationAnswered = false;
+        }
+
+        if (OperatingSystem.IsWindows())
+        {
+            // Windows has no absence check independent of the enumeration: a COM name is not a
+            // filesystem path, so there is no File.Exists equivalent, and the enumeration is itself
+            // the authority (it reads the HARDWARE\DEVICEMAP\SERIALCOMM registry map). That makes
+            // an answered enumeration sufficient — including an empty one, which positively means
+            // no COM ports exist — but leaves a *failed* enumeration with no evidence whatsoever.
+            // Choosing a name in that state would assert against an unverified port and could pass
+            // or fail for reasons unrelated to the translation being tested, so refuse instead.
+            if (!enumerationAnswered)
+            {
+                throw new InvalidOperationException(
+                    "Cannot verify an absent COM name: SerialPort.GetPortNames() failed and Windows " +
+                    "offers no independent check that a COM name is unused. Refusing to assert " +
+                    "against an unverified port.");
+            }
+
+            // The Windows serial stack only accepts COM-prefixed names, so a random suffix is not
+            // an option; take the highest COM number the enumeration does not claim.
+            for (var number = 255; number >= 200; number--)
+            {
+                var candidate = $"COM{number}";
+                if (!enumerated.Contains(candidate))
+                {
+                    return candidate;
+                }
+            }
+
+            throw new InvalidOperationException(
+                "No unused COM name available to exercise the missing-port path.");
+        }
+
+        // Unix needs no such guard: the port name is a filesystem path, so File.Exists answers
+        // independently of the enumeration and a failed enumeration costs the check nothing.
+
+        for (var attempt = 0; attempt < 8; attempt++)
+        {
+            var candidate = $"/dev/tty.daqifi-core-absent-424-{Guid.NewGuid():N}";
+            if (!File.Exists(candidate) && !enumerated.Contains(candidate))
+            {
+                return candidate;
+            }
+        }
+
+        throw new InvalidOperationException(
+            "Could not generate an absent device node path to exercise the missing-port path.");
+    }
+
+    [Fact]
+    public async Task SerialStreamTransport_ConnectAsync_WithMissingPort_ReportsPortNotFound()
+    {
+        // #424: SerialPort.Open reports a port that does not exist as "Access to the port '...' is
+        // denied.", sending users after a permissions problem for what is almost always a typo or
+        // a stale name (USB device nodes are renumbered across replugs).
+        var portName = CreateVerifiedAbsentPort();
+        using var transport = new SerialStreamTransport(portName);
+
+        var ex = await Assert.ThrowsAsync<SerialPortConnectException>(() => transport.ConnectAsync());
+
+        Assert.Equal(SerialPortConnectFailure.NotFound, ex.Reason);
+        Assert.Equal(portName, ex.PortName);
+        Assert.Contains(portName, ex.Message);
+        Assert.Contains("was not found", ex.Message);
+        Assert.DoesNotContain("denied", ex.Message, StringComparison.OrdinalIgnoreCase);
+        Assert.False(transport.IsConnected);
+    }
+
+    [Fact]
+    public async Task SerialStreamTransport_ConnectAsync_WithMissingPort_KeepsThePlatformException()
+    {
+        // The translation adds a name for the failure; it must not throw the diagnosis away.
+        using var transport = new SerialStreamTransport(CreateVerifiedAbsentPort());
+
+        var ex = await Assert.ThrowsAsync<SerialPortConnectException>(() => transport.ConnectAsync());
+
+        Assert.NotNull(ex.InnerException);
+    }
+
+    [Fact]
+    public async Task SerialStreamTransport_ConnectAsync_WithMissingPort_StillCatchableAsIoException()
+    {
+        // The chosen base type: a caller bracketing a connect with catch (IOException) keeps working.
+        using var transport = new SerialStreamTransport(CreateVerifiedAbsentPort());
+
+        var ex = await Assert.ThrowsAnyAsync<IOException>(() => transport.ConnectAsync());
+
+        Assert.IsType<SerialPortConnectException>(ex);
+    }
+
+    [Fact]
+    public async Task SerialStreamTransport_ConnectAsync_WithMissingPort_ReportsTheTypedErrorOnStatusChanged()
+    {
+        // The status event carries the same translated exception, so a subscriber classifying a
+        // failed connect sees the reason rather than the platform's guess.
+        using var transport = new SerialStreamTransport(CreateVerifiedAbsentPort());
+        Exception? reported = null;
+        transport.StatusChanged += (_, e) => reported ??= e.Error;
+
+        await Assert.ThrowsAsync<SerialPortConnectException>(() => transport.ConnectAsync());
+
+        var typed = Assert.IsType<SerialPortConnectException>(reported);
+        Assert.Equal(SerialPortConnectFailure.NotFound, typed.Reason);
+    }
+
+    [Fact]
+    public async Task SerialStreamTransport_ConnectAsync_WhenThePresenceProbeThrows_DoesNotLeakIt()
+    {
+        // The production classification path calls SerialPort.GetPortNames(), which can throw on
+        // some hosts. That must degrade the reason, never replace the connect failure with the
+        // probe's own exception or escape as an unhandled error.
+        using var transport = new SerialStreamTransport(CreateVerifiedAbsentPort())
+        {
+            PortPresenceProbe = _ => throw new UnauthorizedAccessException("the probe could not run")
+        };
+
+        var ex = await Assert.ThrowsAsync<SerialPortConnectException>(() => transport.ConnectAsync());
+
+        // The caller still gets the real open failure, with the platform exception preserved, and
+        // no trace of the probe's failure anywhere in the chain.
+        Assert.NotNull(ex.InnerException);
+        Assert.DoesNotContain("the probe could not run", ex.ToString());
+    }
+
+    [Fact]
+    public async Task SerialStreamTransport_ConnectAsync_WithMissingPort_StillHonorsRetryPolicy()
+    {
+        // Translation happens inside a connect attempt, so it is still just a failed attempt: the
+        // retry loop runs the configured number of times and surfaces the typed exception at the end.
+        using var transport = new SerialStreamTransport(CreateVerifiedAbsentPort());
+        var options = new ConnectionRetryOptions
+        {
+            Enabled = true,
+            MaxAttempts = 3,
+            InitialDelay = TimeSpan.Zero,
+            MaxDelay = TimeSpan.Zero
+        };
+        var failures = 0;
+        transport.StatusChanged += (_, e) => { if (!e.IsConnected) failures++; };
+
+        var ex = await Assert.ThrowsAsync<SerialPortConnectException>(
+            () => transport.ConnectAsync(options));
+
+        Assert.Equal(SerialPortConnectFailure.NotFound, ex.Reason);
+        Assert.Equal(3, failures);
+    }
+
     [Fact]
     public void SerialStreamTransport_Disconnect_WhenNotConnected_ShouldNotThrow()
     {