From 5e5824fa3e6c3d84fa34ce09171031c9ea73acb5 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Sun, 2 Aug 2026 10:49:28 -0600 Subject: [PATCH 1/4] fix(transport): name serial connect failures instead of calling them all access denials (closes #424) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SerialStreamTransport.ConnectAsync forwarded whatever SerialPort.Open threw, so a port that simply does not exist arrived as "Access to the port '' is denied." — pointing at a permissions problem for what is usually a typo or a stale port name, since USB serial device nodes are renumbered across replugs. Open failures are now translated into SerialPortConnectException (an IOException) carrying a typed SerialPortConnectFailure reason — NotFound, InUse, AccessDenied, Unknown — with the original platform exception always preserved as InnerException. This mirrors TcpStreamTransport, which already substitutes a TimeoutException for a misleading TaskCanceledException (daqifi-desktop#517). The reason cannot be recovered from the platform exception. Measured on macOS with System.IO.Ports 10.0.10, a missing port and a port held by another process produce identical exceptions, and the inner IOException's message varied between processes on the same machine for the same port ("Unknown error: 203" vs "No such file or directory") — its HResult is a stale errno, not a usable signal. The reason is therefore derived from evidence gathered at the moment of failure: whether the port is still present (the probe the transport already trusts for unplug detection), and whether a per-user permission gate could apply at all. Platforms that cannot answer degrade to the existing access-denied wording rather than misclassifying. Retry behavior is unchanged; a translated failure is still one failed attempt. Co-Authored-By: Claude Opus 5 --- .../SerialPortConnectExceptionTests.cs | 191 ++++++++++++++++++ .../Transport/SerialStreamTransportTests.cs | 85 ++++++++ .../Transport/SerialPortConnectException.cs | 180 +++++++++++++++++ .../Transport/SerialPortConnectFailure.cs | 42 ++++ .../Transport/SerialStreamTransport.cs | 94 ++++++++- 5 files changed, 591 insertions(+), 1 deletion(-) create mode 100644 src/Daqifi.Core.Tests/Communication/Transport/SerialPortConnectExceptionTests.cs create mode 100644 src/Daqifi.Core/Communication/Transport/SerialPortConnectException.cs create mode 100644 src/Daqifi.Core/Communication/Transport/SerialPortConnectFailure.cs diff --git a/src/Daqifi.Core.Tests/Communication/Transport/SerialPortConnectExceptionTests.cs b/src/Daqifi.Core.Tests/Communication/Transport/SerialPortConnectExceptionTests.cs new file mode 100644 index 00000000..4cda2c3a --- /dev/null +++ b/src/Daqifi.Core.Tests/Communication/Transport/SerialPortConnectExceptionTests.cs @@ -0,0 +1,191 @@ +using Daqifi.Core.Communication.Transport; + +namespace Daqifi.Core.Tests.Communication.Transport; + +/// +/// Pins the connect-failure translation added for #424: 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 +/// UnauthorizedAccessException("Access to the port 'X' is denied.") wrapping an +/// IOException("Unknown error: 203") — identical type, text, and HResult — so the reason has +/// to come from evidence gathered around the failure instead. +/// +public class SerialPortConnectExceptionTests +{ + /// + /// The exact exception shape a failed open produces on macOS/Linux, for every cause. + /// + 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(() => + 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(ex); + } + + [Fact] + public void SerialPortConnectException_RequiresAPortName() + { + Assert.Throws(() => + new SerialPortConnectException(null!, SerialPortConnectFailure.NotFound, "nope")); + } +} diff --git a/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs b/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs index 725b47a9..1362213a 100644 --- a/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs +++ b/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs @@ -141,6 +141,91 @@ public async Task SerialStreamTransport_ConnectAsync_WithInvalidPort_ShouldThrow Assert.False(transport.IsConnected); } + /// + /// A port name that resolves nowhere on the running platform, so the connect fails for the one + /// reason the test cares about. + /// + private static string NonexistentPort => + OperatingSystem.IsWindows() ? "COM254" : "/dev/tty.daqifi-core-nonexistent-424"; + + [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). + using var transport = new SerialStreamTransport(NonexistentPort); + + var ex = await Assert.ThrowsAsync(() => transport.ConnectAsync()); + + Assert.Equal(SerialPortConnectFailure.NotFound, ex.Reason); + Assert.Equal(NonexistentPort, ex.PortName); + Assert.Contains(NonexistentPort, 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(NonexistentPort); + + var ex = await Assert.ThrowsAsync(() => 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(NonexistentPort); + + var ex = await Assert.ThrowsAnyAsync(() => transport.ConnectAsync()); + + Assert.IsType(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(NonexistentPort); + Exception? reported = null; + transport.StatusChanged += (_, e) => reported ??= e.Error; + + await Assert.ThrowsAsync(() => transport.ConnectAsync()); + + var typed = Assert.IsType(reported); + Assert.Equal(SerialPortConnectFailure.NotFound, typed.Reason); + } + + [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(NonexistentPort); + 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( + () => transport.ConnectAsync(options)); + + Assert.Equal(SerialPortConnectFailure.NotFound, ex.Reason); + Assert.Equal(3, failures); + } + [Fact] public void SerialStreamTransport_Disconnect_WhenNotConnected_ShouldNotThrow() { diff --git a/src/Daqifi.Core/Communication/Transport/SerialPortConnectException.cs b/src/Daqifi.Core/Communication/Transport/SerialPortConnectException.cs new file mode 100644 index 00000000..6d10bd48 --- /dev/null +++ b/src/Daqifi.Core/Communication/Transport/SerialPortConnectException.cs @@ -0,0 +1,180 @@ +namespace Daqifi.Core.Communication.Transport; + +/// +/// Thrown when +/// cannot open its serial port, carrying a stable that says why. +/// +/// +/// +/// What this replaces. reports a missing port +/// on macOS and Linux as UnauthorizedAccessException: Access to the port '...' is denied., +/// which sends users looking for a permissions problem when the real cause is a wrong or stale +/// port name — the common case, because USB serial device nodes are renumbered across replugs. +/// This is the serial analog of the that +/// substitutes for a misleading TaskCanceledException +/// (daqifi-desktop#517): the same failure, named accurately. +/// +/// +/// Why a typed reason. The platform exception cannot be classified after the fact. Measured +/// on macOS with System.IO.Ports 10.0.10, a missing port and a port held by another process +/// produce byte-identical exceptions — same type, same message, and an inner +/// with the same text and the same (bogus, non-errno) +/// . Matching on message text or on the inner exception is +/// therefore not merely unportable, it does not work at all. is derived from +/// evidence gathered around the failure instead — see +/// . +/// +/// +/// Base type. Derives from : failing to open an I/O device is an +/// I/O error, and on Windows a non-existent port already surfaces as an +/// -derived exception, so a caller that brackets a connect with +/// catch (IOException) keeps working. The original platform exception is always preserved +/// as . +/// +/// +/// Retries. This type does not change retry behavior — a failed open is still one failed +/// attempt under . It does let a caller decide whether +/// retrying is worthwhile: can clear on its own, +/// while and +/// will not. +/// +/// +public class SerialPortConnectException : IOException +{ + /// + /// Gets the name of the port that could not be opened (for example COM3 or + /// /dev/cu.usbmodem1101). + /// + public string PortName { get; } + + /// + /// Gets the classified reason the port could not be opened. + /// + public SerialPortConnectFailure Reason { get; } + + /// + /// Initializes a new instance of the class. + /// + /// The port that could not be opened. + /// The classified reason for the failure. + /// The message that describes the failure. + /// + /// The platform exception that caused the failure, or null. + /// + /// Thrown when is null. + public SerialPortConnectException( + string portName, + SerialPortConnectFailure reason, + string message, + Exception? innerException = null) + : base(message, innerException) + { + PortName = portName ?? throw new ArgumentNullException(nameof(portName)); + Reason = reason; + } + + /// + /// Builds the translated exception for a failed , + /// preserving as the inner exception. + /// + /// The port that could not be opened. + /// The platform exception the open threw. + /// + /// Whether the port is still visible to the system, or null if that could not be + /// observed. See . + /// + /// + /// Whether a permission failure has been positively ruled out, or null if unknown. + /// See . + /// + internal static SerialPortConnectException FromOpenFailure( + string portName, + Exception error, + bool? portPresent, + bool? permissionGateRuledOut) + { + var reason = Classify(error, portPresent, permissionGateRuledOut); + return new SerialPortConnectException(portName, reason, DescribeFailure(portName, reason), error); + } + + /// + /// Classifies a failed serial open from evidence collected around it, rather than from the + /// platform exception's type or text. + /// + /// The platform exception the open threw. + /// + /// true if the port is still enumerated (or its device node still exists), + /// false if it is definitely absent, null if the probe could not answer. A + /// failure to observe is never treated as absence. + /// + /// + /// true when the platform cannot be denying access on permission grounds — a Unix + /// device node that grants read and write to every user, or a Windows COM port, which has no + /// equivalent per-user gate. false when a permission failure is possible, null + /// when it could not be determined. + /// + /// The classified reason. + /// + /// + /// The order matters. An explicit — what Windows reports + /// for a port that no longer exists — is conclusive on its own. Otherwise absence of the port + /// is the deciding signal, and it is the one that fixes the reported bug: it is portable, it + /// does not read any exception text, and it is the same probe the transport already trusts to + /// detect an unplug on an established connection. + /// + /// + /// Only once the port is known (or assumed) to exist does an access-denied exception have to be + /// split between "someone else has it" and "you may not have it", and that split is the one + /// piece the platform genuinely cannot answer on macOS. It is resolved from + /// , and when that is unavailable the result stays + /// — the status-quo wording — so an + /// unfamiliar platform degrades to today's behavior instead of asserting something false. + /// + /// + internal static SerialPortConnectFailure Classify( + Exception error, + bool? portPresent, + bool? permissionGateRuledOut) + { + ArgumentNullException.ThrowIfNull(error); + + // Windows names the missing-port case outright. Checked before anything else, and through + // the inner exception too, because it needs no corroboration. + if (error is FileNotFoundException || error.InnerException is FileNotFoundException) + { + return SerialPortConnectFailure.NotFound; + } + + // The port is definitely gone. This is the case the issue is about, and the only one where + // "access is denied" was actively misleading. + if (portPresent == false) + { + return SerialPortConnectFailure.NotFound; + } + + if (error is UnauthorizedAccessException || error.InnerException is UnauthorizedAccessException) + { + return permissionGateRuledOut == true + ? SerialPortConnectFailure.InUse + : SerialPortConnectFailure.AccessDenied; + } + + return SerialPortConnectFailure.Unknown; + } + + /// + /// Produces the user-facing message for a classified failure. + /// + /// The port that could not be opened. + /// The classified reason. + /// A single-sentence description naming the port. + internal static string DescribeFailure(string portName, SerialPortConnectFailure reason) => reason switch + { + SerialPortConnectFailure.NotFound => $"Serial port '{portName}' was not found.", + SerialPortConnectFailure.InUse => $"Serial port '{portName}' is in use.", + // Deliberately the framework's own wording for the one case where it was always accurate, + // so anything already keying on that phrasing for a real permission problem still matches. + SerialPortConnectFailure.AccessDenied => $"Access to the port '{portName}' is denied.", + _ => $"Serial port '{portName}' could not be opened." + }; +} diff --git a/src/Daqifi.Core/Communication/Transport/SerialPortConnectFailure.cs b/src/Daqifi.Core/Communication/Transport/SerialPortConnectFailure.cs new file mode 100644 index 00000000..e5a323ec --- /dev/null +++ b/src/Daqifi.Core/Communication/Transport/SerialPortConnectFailure.cs @@ -0,0 +1,42 @@ +namespace Daqifi.Core.Communication.Transport; + +/// +/// Why a serial port could not be opened, as reported by +/// . +/// +/// +/// This is the stable, typed alternative to inspecting the platform exception a failed +/// produces. That exception carries no reliable +/// classification of its own: on macOS a missing port and a busy port both surface as +/// ("Access to the port '...' is denied.") wrapping an +/// whose message and are the same in +/// both cases, so neither the type, the text, nor the errno can tell them apart. +/// +public enum SerialPortConnectFailure +{ + /// + /// The reason could not be determined. The platform exception is still available as + /// . + /// + Unknown = 0, + + /// + /// The port does not exist. Usually a typo or a stale port name — USB serial device nodes are + /// renumbered across replugs, so a name captured earlier in a session may no longer resolve. + /// Retrying does not help; re-enumerate the available ports instead. + /// + NotFound, + + /// + /// The port exists but another process already holds it open. Retrying can succeed once the + /// other process releases the port. + /// + InUse, + + /// + /// The port exists but the current process is not permitted to open it — for example a Linux + /// device node owned by a group the user is not a member of (commonly dialout). + /// Retrying does not help until the permission is granted. + /// + AccessDenied +} diff --git a/src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs b/src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs index 507104d9..23ed41e0 100644 --- a/src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs +++ b/src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs @@ -226,6 +226,10 @@ public string ConnectionInfo /// Establishes the serial connection asynchronously. /// /// A task representing the asynchronous connect operation. + /// + /// Thrown when the port cannot be opened, with + /// naming the cause. + /// public async Task ConnectAsync() { await ConnectAsync(null); @@ -236,6 +240,10 @@ public async Task ConnectAsync() /// /// Configuration for retry behavior. If null, uses default single attempt. /// A task representing the asynchronous connect operation. + /// + /// Thrown when the final attempt cannot open the port, with + /// naming the cause. + /// public async Task ConnectAsync(ConnectionRetryOptions? retryOptions) { await ConnectAsync(retryOptions, CancellationToken.None); @@ -249,12 +257,28 @@ public async Task ConnectAsync(CancellationToken cancellationToken) /// /// + /// /// Cancellation is observed between attempts and while waiting out a backoff delay, and is /// checked immediately before each . It cannot interrupt the /// Open call itself — the framework offers no cancellable form of it — so on a port that /// hangs open, cancellation takes effect when that call returns. In practice opening a serial /// port either succeeds or fails quickly; the long wait worth cancelling is the retry loop. + /// + /// + /// A failed is reported as a + /// naming the port and why it could not be opened, + /// rather than the platform's own exception, which on macOS and Linux calls every failure — + /// including a port that simply does not exist — an access denial (#424). The original is kept + /// as the inner exception. The reason is decided from what can be observed at the moment of + /// the failure: whether the port is still present, and whether the platform could be applying + /// a permission gate at all. Retry behavior is unchanged; a translated failure is still one + /// failed attempt. + /// /// + /// + /// Thrown when the final attempt cannot open the port, with + /// naming the cause. + /// public async Task ConnectAsync(ConnectionRetryOptions? retryOptions, CancellationToken cancellationToken) { ThrowIfDisposed(); @@ -280,7 +304,21 @@ await ConnectRetryExecutor.ExecuteAsync( // honor a cancel that arrived while the previous attempt was backing off. attemptToken.ThrowIfCancellationRequested(); - _serialPort.Open(); + try + { + _serialPort.Open(); + } + catch (Exception ex) when (ex is UnauthorizedAccessException or IOException) + { + // Name the failure instead of forwarding the platform's guess at it (#424). + // The evidence is gathered here, immediately after the failed open, because it + // is only meaningful in that moment; the classification itself lives in + // SerialPortConnectException.Classify. Argument and state exceptions are left + // alone — a bad baud rate or an already-open port is a caller bug, not a port + // that could not be opened. + throw SerialPortConnectException.FromOpenFailure( + _portName, ex, TryObservePortPresence(), TryRuleOutPermissionGate()); + } // After a successful open, swap the connect timeouts for the (shorter) // operational ones — both directions, not just reads (#399). @@ -441,6 +479,60 @@ internal void StartDropDetection() _livenessCheckInterval); } + /// + /// Observes whether the port exists, for classifying a failed open. Returns null when + /// the probe could not answer — a failure to observe is not evidence of absence, and reporting + /// it as absence would turn an unrelated connect failure into a bogus "port was not found". + /// + private bool? TryObservePortPresence() + { + try + { + return IsPortPresent(); + } + catch (Exception) + { + return null; + } + } + + /// + /// Reports whether the platform can be ruled out as the source of an access-denied failure, + /// which is what separates a port held by another process from one the caller may not open. + /// + /// + /// true when no per-user permission gate can apply, false when one could, + /// null when it could not be determined. + /// + /// + /// A Unix device node that grants read and write to every user cannot produce a permission + /// failure for anybody, so a denial on such a node is exclusivity — the case macOS reports with + /// the same exception it uses for every other open failure, and /dev/cu.* nodes there are + /// crw-rw-rw-. A node that does not grant that (a Linux dialout-owned port at + /// crw-rw----, say) keeps the access-denied reading, so the genuine permission case is + /// still reported as one. Windows COM ports have no comparable gate: a port that exists and + /// refuses to open is one another process holds. + /// + private bool? TryRuleOutPermissionGate() + { + if (OperatingSystem.IsWindows()) + { + return true; + } + + try + { + var mode = File.GetUnixFileMode(_portName); + return mode.HasFlag(UnixFileMode.OtherRead) && mode.HasFlag(UnixFileMode.OtherWrite); + } + catch (Exception) + { + // No stat (the port is gone, the name is not a path, or the platform will not say). + // Unknown, never a guess: the caller falls back to the access-denied wording. + return null; + } + } + /// /// Reports whether the configured port is still present on the system. /// From 0d96decdc09cbe71ca4a39e883e14c92858f437b Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Sun, 2 Aug 2026 12:50:32 -0600 Subject: [PATCH 2/4] test(transport): generate a verified-absent port name; document the connect exception change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Qodo review on #427. The missing-port tests hard-coded their absent port name, which is only absent by assumption — a host with a virtual COM port or a leftover device node would have made them open real hardware or fail with a different error shape. The name is now generated and checked at runtime: a GUID-suffixed device node on Unix, and on Windows the highest COM number SerialPort.GetPortNames() does not claim, since the Windows serial stack only accepts COM-prefixed names. Each test captures the name in a local, which the previous property-per-call form could not do safely. Also documents the deliberate behavioral change for callers: a connect that used to throw UnauthorizedAccessException now throws SerialPortConnectException, so that catch no longer matches, with the migration spelled out on both ConnectAsync and the exception type. No runtime behavior changes in this commit. Co-Authored-By: Claude Opus 5 --- .../Transport/SerialStreamTransportTests.cs | 59 +++++++++++++++---- .../Transport/SerialPortConnectException.cs | 7 +++ .../Transport/SerialStreamTransport.cs | 14 +++++ 3 files changed, 69 insertions(+), 11 deletions(-) diff --git a/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs b/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs index 1362213a..d4e4a903 100644 --- a/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs +++ b/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs @@ -142,11 +142,47 @@ public async Task SerialStreamTransport_ConnectAsync_WithInvalidPort_ShouldThrow } /// - /// A port name that resolves nowhere on the running platform, so the connect fails for the one - /// reason the test cares about. + /// Produces a port name checked to be absent on the running host, so the missing-port tests + /// assert the translation and nothing else. /// - private static string NonexistentPort => - OperatingSystem.IsWindows() ? "COM254" : "/dev/tty.daqifi-core-nonexistent-424"; + /// + /// 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. + /// + private static string CreateVerifiedAbsentPort() + { + var enumerated = new HashSet(SerialPort.GetPortNames(), StringComparer.OrdinalIgnoreCase); + + if (OperatingSystem.IsWindows()) + { + // 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."); + } + + 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() @@ -154,13 +190,14 @@ public async Task SerialStreamTransport_ConnectAsync_WithMissingPort_ReportsPort // #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). - using var transport = new SerialStreamTransport(NonexistentPort); + var portName = CreateVerifiedAbsentPort(); + using var transport = new SerialStreamTransport(portName); var ex = await Assert.ThrowsAsync(() => transport.ConnectAsync()); Assert.Equal(SerialPortConnectFailure.NotFound, ex.Reason); - Assert.Equal(NonexistentPort, ex.PortName); - Assert.Contains(NonexistentPort, ex.Message); + 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); @@ -170,7 +207,7 @@ public async Task SerialStreamTransport_ConnectAsync_WithMissingPort_ReportsPort 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(NonexistentPort); + using var transport = new SerialStreamTransport(CreateVerifiedAbsentPort()); var ex = await Assert.ThrowsAsync(() => transport.ConnectAsync()); @@ -181,7 +218,7 @@ public async Task SerialStreamTransport_ConnectAsync_WithMissingPort_KeepsThePla 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(NonexistentPort); + using var transport = new SerialStreamTransport(CreateVerifiedAbsentPort()); var ex = await Assert.ThrowsAnyAsync(() => transport.ConnectAsync()); @@ -193,7 +230,7 @@ public async Task SerialStreamTransport_ConnectAsync_WithMissingPort_ReportsTheT { // 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(NonexistentPort); + using var transport = new SerialStreamTransport(CreateVerifiedAbsentPort()); Exception? reported = null; transport.StatusChanged += (_, e) => reported ??= e.Error; @@ -208,7 +245,7 @@ public async Task SerialStreamTransport_ConnectAsync_WithMissingPort_StillHonors { // 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(NonexistentPort); + using var transport = new SerialStreamTransport(CreateVerifiedAbsentPort()); var options = new ConnectionRetryOptions { Enabled = true, diff --git a/src/Daqifi.Core/Communication/Transport/SerialPortConnectException.cs b/src/Daqifi.Core/Communication/Transport/SerialPortConnectException.cs index 6d10bd48..51cfee14 100644 --- a/src/Daqifi.Core/Communication/Transport/SerialPortConnectException.cs +++ b/src/Daqifi.Core/Communication/Transport/SerialPortConnectException.cs @@ -32,6 +32,13 @@ namespace Daqifi.Core.Communication.Transport; /// as . /// /// +/// Migrating. A connect that previously threw +/// now throws this instead, so catch (UnauthorizedAccessException) no longer matches. +/// Replace it with catch (SerialPortConnectException ex) and switch on +/// — that distinction is the thing the old exception could not express, since on macOS a missing +/// port and a busy port threw the very same exception. +/// +/// /// Retries. This type does not change retry behavior — a failed open is still one failed /// attempt under . It does let a caller decide whether /// retrying is worthwhile: can clear on its own, diff --git a/src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs b/src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs index 23ed41e0..78468b69 100644 --- a/src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs +++ b/src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs @@ -274,6 +274,20 @@ public async Task ConnectAsync(CancellationToken cancellationToken) /// a permission gate at all. Retry behavior is unchanged; a translated failure is still one /// failed attempt. /// + /// + /// Behavioral change for callers. This method used to surface whatever + /// threw — on macOS and Linux always an + /// whatever the real cause, on Windows a + /// or an . Those + /// are now wrapped, so catch (UnauthorizedAccessException) around a connect no longer + /// matches and any branching on the platform exception's type has to move. Catch + /// and switch on + /// , which is the classification the platform + /// exception never reliably carried; catch (IOException) still matches for callers that + /// only need the broad case. The original exception remains available as + /// , so nothing is lost — only the type that arrives at + /// the catch site changes. + /// /// /// /// Thrown when the final attempt cannot open the port, with From ff877f8d945bca4576a80eed1c026f929819b658 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Sun, 2 Aug 2026 12:58:58 -0600 Subject: [PATCH 3/4] test(transport): guard the test helper's port enumeration and pin the production guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Qodo round 2 on #427. CreateVerifiedAbsentPort() called SerialPort.GetPortNames() unguarded, so a host where enumeration throws (a container without /dev access, a locked-down machine) would have failed these tests from inside the helper — which reads as the feature under test breaking rather than the environment. An enumeration that cannot answer now contributes no names; File.Exists still gives an independent absence check on Unix, and an unclaimed high COM number remains the best answer on Windows. The production classification path was already guarded — TryObservePortPresence catches everything and returns null, which Classify treats as unknown presence and degrades to the access-denied wording rather than claiming NotFound on no evidence. That contract was untested, so it is now pinned via the PortPresenceProbe seam: a throwing probe must not replace the connect failure or escape. Test-only; no runtime behavior change. Co-Authored-By: Claude Opus 5 --- .../Transport/SerialStreamTransportTests.cs | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs b/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs index d4e4a903..0e70d9b4 100644 --- a/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs +++ b/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs @@ -152,7 +152,21 @@ public async Task SerialStreamTransport_ConnectAsync_WithInvalidPort_ShouldThrow /// private static string CreateVerifiedAbsentPort() { - var enumerated = new HashSet(SerialPort.GetPortNames(), StringComparer.OrdinalIgnoreCase); + // SerialPort.GetPortNames() can itself throw (a container without /dev access, a + // locked-down host). It must not take the suite down from inside a test helper: that would + // surface as these tests failing, which reads exactly like the feature under test broke. + // An enumeration that cannot answer simply contributes no names — on Unix File.Exists + // still gives an independent absence check, and on Windows an unclaimed high COM number + // remains the best available answer. + HashSet enumerated; + try + { + enumerated = new HashSet(SerialPort.GetPortNames(), StringComparer.OrdinalIgnoreCase); + } + catch (Exception) + { + enumerated = new HashSet(StringComparer.OrdinalIgnoreCase); + } if (OperatingSystem.IsWindows()) { @@ -240,6 +254,25 @@ public async Task SerialStreamTransport_ConnectAsync_WithMissingPort_ReportsTheT 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(() => 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() { From c4e328e62db4877cf6f8a4d5992137b499126edc Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Sun, 2 Aug 2026 13:07:45 -0600 Subject: [PATCH 4/4] test(transport): never choose a COM name the enumeration could not vouch for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Qodo round 3 on #427. The round-2 fail-soft fix collapsed two states that are not equivalent: an enumeration that returned nothing, and one that threw. On Windows an empty result is a real answer — it means no COM ports exist, so any COM name is absent — while a throw is no evidence at all. Treating both as "no names claimed" let the Windows branch pick a COM name backed by nothing. The two are now tracked separately. An answered enumeration is sufficient on Windows because it is itself the authority there (it reads HARDWARE\DEVICEMAP\SERIALCOMM), and Windows has no independent absence check the way a Unix device node has File.Exists. A failed enumeration now refuses with a precise diagnostic rather than asserting against an unverified port. Unix is unchanged and needs no guard: the port name is a filesystem path, so File.Exists answers independently of the enumeration. Test-only; no runtime behavior change. Co-Authored-By: Claude Opus 5 --- .../Transport/SerialStreamTransportTests.cs | 33 +++++++++++++++---- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs b/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs index 0e70d9b4..0a6e7ca8 100644 --- a/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs +++ b/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs @@ -152,24 +152,42 @@ public async Task SerialStreamTransport_ConnectAsync_WithInvalidPort_ShouldThrow /// private static string CreateVerifiedAbsentPort() { - // SerialPort.GetPortNames() can itself throw (a container without /dev access, a - // locked-down host). It must not take the suite down from inside a test helper: that would - // surface as these tests failing, which reads exactly like the feature under test broke. - // An enumeration that cannot answer simply contributes no names — on Unix File.Exists - // still gives an independent absence check, and on Windows an unclaimed high COM number - // remains the best available answer. + // 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 enumerated; try { enumerated = new HashSet(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(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--) @@ -185,6 +203,9 @@ private static string CreateVerifiedAbsentPort() "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}";