From 0a455a324d76eac8059f750f358cfb8ae277b011 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=2EB=C3=B6xler?= Date: Mon, 2 Feb 2026 13:58:19 +0100 Subject: [PATCH] fix(listener): keep UI inactive and show MessageBox when UDP bind fails - Add StartListeningAsync returning StartListeningResult; only set IsListening when at least one port started - Use AsyncRelayCommand for Start Listening; show MessageBox on failure with error details - Add PortProcessResolver (Windows) to show process name and PID when port is in use - Add StartListeningResult model, Error_StartingListenerCaption and Status_* resources, UdpLogReceiverServiceTests Fixes #142 --- .../Models/StartListeningResult.cs | 28 ++++ .../Resources/Resources.resx | 12 ++ .../Services/PortProcessResolver.cs | 138 ++++++++++++++++++ .../Services/UdpLogReceiverService.cs | 56 ++++++- .../ViewModels/MainViewModel.cs | 41 ++++-- .../Services/UdpLogReceiverServiceTests.cs | 73 +++++++++ ui/Sentinel.NLogViewer.Wpf/RelayCommand.cs | 74 ++++++++++ 7 files changed, 407 insertions(+), 15 deletions(-) create mode 100644 app/Sentinel.NLogViewer.App/Models/StartListeningResult.cs create mode 100644 app/Sentinel.NLogViewer.App/Services/PortProcessResolver.cs create mode 100644 tests/Sentinel.NLogViewer.App.Tests/Services/UdpLogReceiverServiceTests.cs diff --git a/app/Sentinel.NLogViewer.App/Models/StartListeningResult.cs b/app/Sentinel.NLogViewer.App/Models/StartListeningResult.cs new file mode 100644 index 0000000..f91e155 --- /dev/null +++ b/app/Sentinel.NLogViewer.App/Models/StartListeningResult.cs @@ -0,0 +1,28 @@ +namespace Sentinel.NLogViewer.App.Models; + +/// +/// Result of starting the UDP listener(s). +/// +public sealed class StartListeningResult +{ + /// + /// Whether at least one listener was started successfully. + /// + public bool AnyStarted { get; } + + /// + /// Aggregated error message for failed addresses; when port is in use, may include process name and PID. + /// + public string ErrorMessage { get; } + + /// + /// Creates a new instance. + /// + /// True if at least one listener started. + /// Error message for failures (e.g. address already in use, process name/PID). + public StartListeningResult(bool anyStarted, string errorMessage) + { + AnyStarted = anyStarted; + ErrorMessage = errorMessage ?? string.Empty; + } +} diff --git a/app/Sentinel.NLogViewer.App/Resources/Resources.resx b/app/Sentinel.NLogViewer.App/Resources/Resources.resx index 13446d5..0e10014 100644 --- a/app/Sentinel.NLogViewer.App/Resources/Resources.resx +++ b/app/Sentinel.NLogViewer.App/Resources/Resources.resx @@ -103,5 +103,17 @@ Stopped + + Error + + + (Some ports could not be opened.) + + + Error starting listener + + + Error starting listener + diff --git a/app/Sentinel.NLogViewer.App/Services/PortProcessResolver.cs b/app/Sentinel.NLogViewer.App/Services/PortProcessResolver.cs new file mode 100644 index 0000000..8ec3524 --- /dev/null +++ b/app/Sentinel.NLogViewer.App/Services/PortProcessResolver.cs @@ -0,0 +1,138 @@ +using System; +using System.Diagnostics; +using System.Globalization; +using System.IO; +using System.Text; +using System.Text.RegularExpressions; + +namespace Sentinel.NLogViewer.App.Services; + +/// +/// Resolves the process (name and PID) that is using a given port on Windows. +/// +public static class PortProcessResolver +{ + /// + /// Tries to get the process name and PID that is using the given port. + /// Only supported on Windows; uses netstat output parsing. + /// + /// Port number (e.g. 4000). + /// True for UDP, false for TCP. + /// Process name and PID if found; (null, null) on non-Windows, parse failure, or when process has exited. + public static (string? processName, int? pid) TryGetProcessUsingPort(int port, bool udp = true) + { + if (!OperatingSystem.IsWindows()) + return (null, null); + + try + { + var pid = TryGetPidFromNetStat(port, udp); + if (pid == null) + return (null, null); + + string? processName = null; + try + { + using var process = Process.GetProcessById(pid.Value); + processName = process.ProcessName; + } + catch (ArgumentException) + { + // Process may have exited or access denied + } + + return (processName, pid); + } + catch + { + return (null, null); + } + } + + private static int? TryGetPidFromNetStat(int port, bool udp) + { + try + { + var netstatPath = Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.System), + "netstat.exe"); + if (!File.Exists(netstatPath)) + netstatPath = "netstat.exe"; + + using var process = new Process(); + process.StartInfo = new ProcessStartInfo + { + FileName = netstatPath, + Arguments = "-a -n -o", + UseShellExecute = false, + RedirectStandardOutput = true, + RedirectStandardError = true, + CreateNoWindow = true, + // Netstat outputs in the console/OEM code page on Windows + StandardOutputEncoding = GetConsoleOutputEncoding(), + StandardErrorEncoding = GetConsoleOutputEncoding() + }; + process.Start(); + var output = process.StandardOutput.ReadToEnd() + process.StandardError.ReadToEnd(); + process.WaitForExit(5000); + + var protocol = udp ? "UDP" : "TCP"; + // Match local address containing :port (e.g. 0.0.0.0:4000 or [::]:4000) + var portPattern = $":{port}\\b"; + var lines = output.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries); + + foreach (var line in lines) + { + var trimmed = line.TrimStart(); + if (!trimmed.StartsWith(protocol, StringComparison.OrdinalIgnoreCase)) + continue; + if (!Regex.IsMatch(line, portPattern)) + continue; + + // Last column is PID; extract last integer on the line + var pid = ExtractLastPidFromLine(line); + if (pid != null) + return pid; + } + } + catch + { + // Ignore + } + + return null; + } + + /// + /// Extracts the last integer (PID) from a netstat line. Handles variable spacing. + /// + private static int? ExtractLastPidFromLine(string line) + { + var tokens = Regex.Split(line.Trim(), @"\s+"); + for (var i = tokens.Length - 1; i >= 0; i--) + { + if (string.IsNullOrEmpty(tokens[i])) + continue; + if (int.TryParse(tokens[i], NumberStyles.None, CultureInfo.InvariantCulture, out var pid) && pid > 0) + return pid; + break; + } + return null; + } + + private static Encoding GetConsoleOutputEncoding() + { + if (!OperatingSystem.IsWindows()) + return Encoding.UTF8; + try + { + // Netstat outputs in the console (OEM) code page on Windows + var oemCodePage = CultureInfo.CurrentCulture.TextInfo.OEMCodePage; + return Encoding.GetEncoding(oemCodePage); + } + catch + { + return Encoding.Default; + } + } +} diff --git a/app/Sentinel.NLogViewer.App/Services/UdpLogReceiverService.cs b/app/Sentinel.NLogViewer.App/Services/UdpLogReceiverService.cs index 0aab78e..9f16a36 100644 --- a/app/Sentinel.NLogViewer.App/Services/UdpLogReceiverService.cs +++ b/app/Sentinel.NLogViewer.App/Services/UdpLogReceiverService.cs @@ -15,6 +15,8 @@ namespace Sentinel.NLogViewer.App.Services; /// public class UdpLogReceiverService(Log4JEventParser xmlParser) : IDisposable { + private const int WSAEADDRINUSE = 10048; + private readonly List _udpClients = new(); private readonly List _cancellationTokens = new(); private readonly Log4JEventParser _xmlParser = xmlParser ?? throw new ArgumentNullException(nameof(xmlParser)); @@ -23,15 +25,25 @@ public class UdpLogReceiverService(Log4JEventParser xmlParser) : IDisposable public IObservable Log4JEventObservable => _log4JEventObservable; private readonly Subject _log4JEventObservable = new(); - public void StartListening(List addresses) + /// + /// Starts listening on the given UDP addresses. Returns a result indicating success and any error messages. + /// + /// List of addresses in format udp://host:port (e.g. udp://0.0.0.0:4000). + /// Optional cancellation token. + /// Result with AnyStarted and aggregated ErrorMessage (e.g. including process name/PID when port is in use). + public async Task StartListeningAsync(IReadOnlyList addresses, CancellationToken cancellationToken = default) { StopListening(); + var errors = new List(); foreach (var address in addresses) { + if (cancellationToken.IsCancellationRequested) + break; + var uri = new Uri(address); + try { - var uri = new Uri(address); if (uri.Scheme != "udp") continue; @@ -39,18 +51,50 @@ public void StartListening(List addresses) var udpClient = new UdpClient(port); _udpClients.Add(udpClient); - var cts = new CancellationTokenSource(); + var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); _cancellationTokens.Add(cts); - // Start receiving on this port Task.Run(() => ReceiveLoop(udpClient, port, cts.Token), cts.Token); } + catch (SocketException ex) when (ex.SocketErrorCode == SocketError.AddressAlreadyInUse || ex.NativeErrorCode == WSAEADDRINUSE) + { + var msg = $"Error starting listener on {address}: {ex.Message}"; + var portForResolver = GetPortFromAddress(address); + if (portForResolver != null) + { + var (processName, pid) = PortProcessResolver.TryGetProcessUsingPort(portForResolver.Value, udp: true); + if (pid != null) + msg += processName != null + ? $"\n\nProcess using port {uri.Port}: {processName} (PID: {pid})" + : $"\n\nPort {uri.Port} in use by process PID: {pid}"; + } + errors.Add(msg); + System.Diagnostics.Debug.WriteLine(msg); + } catch (Exception ex) { - // Log error but continue with other ports - System.Diagnostics.Debug.WriteLine($"Error starting listener on {address}: {ex.Message}"); + var msg = $"Error starting listener on {address}: {ex.Message}"; + errors.Add(msg); + System.Diagnostics.Debug.WriteLine(msg); } } + + var anyStarted = _udpClients.Count > 0; + var errorMessage = errors.Count > 0 ? string.Join(Environment.NewLine, errors) : string.Empty; + return await Task.FromResult(new StartListeningResult(anyStarted, errorMessage)); + } + + private static int? GetPortFromAddress(string address) + { + try + { + var uri = new Uri(address); + return uri.Port; + } + catch + { + return null; + } } public void StopListening() diff --git a/app/Sentinel.NLogViewer.App/ViewModels/MainViewModel.cs b/app/Sentinel.NLogViewer.App/ViewModels/MainViewModel.cs index 65d5ea9..1f89f18 100644 --- a/app/Sentinel.NLogViewer.App/ViewModels/MainViewModel.cs +++ b/app/Sentinel.NLogViewer.App/ViewModels/MainViewModel.cs @@ -61,7 +61,7 @@ public MainViewModel( LogTabs = new ObservableCollection(); // Initialize commands - StartListeningCommand = new RelayCommand(StartListening, () => !_isListening); + StartListeningCommand = new AsyncRelayCommand(StartListeningAsync, () => !_isListening); StopListeningCommand = new RelayCommand(StopListening, () => _isListening); OpenFileCommand = new RelayCommand(OpenFile); OpenSettingsCommand = new RelayCommand(OpenSettings); @@ -153,7 +153,7 @@ private set { _isListening = value; OnPropertyChanged(); - ((RelayCommand)StartListeningCommand).RaiseCanExecuteChanged(); + ((AsyncRelayCommand)StartListeningCommand).RaiseCanExecuteChanged(); ((RelayCommand)StopListeningCommand).RaiseCanExecuteChanged(); } } @@ -269,25 +269,48 @@ private void LoadConfiguration() Task.Run(async () => { await Task.Delay(500); // Small delay to ensure UI is ready - System.Windows.Application.Current.Dispatcher.Invoke(StartListening); + System.Windows.Application.Current.Dispatcher.Invoke(() => _ = StartListeningAsync()); }); } } - private void StartListening() + /// + /// Starts the UDP listener asynchronously. On failure (e.g. port in use), shows a MessageBox and keeps the listener inactive. + /// + private async Task StartListeningAsync() { try { var config = _configService.LoadConfiguration(); - _udpReceiverService.StartListening(config.Ports); - IsListening = true; - ListeningStatus = _localizationService.GetString("Status_Listening", "Listening"); - StatusMessage = _localizationService.GetString("Status_ListeningOnPorts", $"Listening on {config.Ports.Count} port(s)"); + var result = await _udpReceiverService.StartListeningAsync(config.Ports); + + if (result.AnyStarted) + { + IsListening = true; + ListeningStatus = _localizationService.GetString("Status_Listening", "Listening"); + StatusMessage = _localizationService.GetString("Status_ListeningOnPorts", $"Listening on {config.Ports.Count} port(s)"); + if (!string.IsNullOrEmpty(result.ErrorMessage)) + { + // Partial failure: some ports failed + StatusMessage += " " + _localizationService.GetString("Status_SomePortsFailed", "(Some ports could not be opened.)"); + } + } + else + { + IsListening = false; + ListeningStatus = _localizationService.GetString("Status_Error", "Error"); + StatusMessage = result.ErrorMessage; + var caption = _localizationService.GetString("Error_StartingListenerCaption", "Error starting listener"); + MessageBox.Show(result.ErrorMessage, caption, MessageBoxButton.OK, MessageBoxImage.Error); + } } catch (Exception ex) { - StatusMessage = _localizationService.GetString("Error_StartingListener", $"Error starting listener: {ex.Message}"); + IsListening = false; ListeningStatus = _localizationService.GetString("Status_Error", "Error"); + StatusMessage = _localizationService.GetString("Error_StartingListener", $"Error starting listener: {ex.Message}"); + var caption = _localizationService.GetString("Error_StartingListenerCaption", "Error starting listener"); + MessageBox.Show(ex.Message, caption, MessageBoxButton.OK, MessageBoxImage.Error); } } diff --git a/tests/Sentinel.NLogViewer.App.Tests/Services/UdpLogReceiverServiceTests.cs b/tests/Sentinel.NLogViewer.App.Tests/Services/UdpLogReceiverServiceTests.cs new file mode 100644 index 0000000..c2757db --- /dev/null +++ b/tests/Sentinel.NLogViewer.App.Tests/Services/UdpLogReceiverServiceTests.cs @@ -0,0 +1,73 @@ +using System.Collections.Generic; +using System.Net; +using System.Net.Sockets; +using System.Threading.Tasks; +using Sentinel.NLogViewer.App.Parsers; +using Sentinel.NLogViewer.App.Services; +using Xunit; + +namespace Sentinel.NLogViewer.App.Tests.Services; + +/// +/// Tests for StartListeningAsync result and error handling. +/// +public class UdpLogReceiverServiceTests +{ + private static UdpLogReceiverService CreateService() + { + return new UdpLogReceiverService(new Log4JEventParser()); + } + + [Fact] + public async Task StartListeningAsync_EmptyAddresses_ReturnsNoStartedAndEmptyError() + { + using var service = CreateService(); + var result = await service.StartListeningAsync(new List()); + + Assert.False(result.AnyStarted); + Assert.Equal(string.Empty, result.ErrorMessage); + } + + [Fact] + public async Task StartListeningAsync_NonUdpAddresses_SkipsAndReturnsNoStarted() + { + using var service = CreateService(); + var addresses = new List { "http://0.0.0.0:4000", "https://localhost:5000" }; + var result = await service.StartListeningAsync(addresses); + + Assert.False(result.AnyStarted); + Assert.Equal(string.Empty, result.ErrorMessage); + } + + [Fact] + public async Task StartListeningAsync_PortInUse_ReturnsNoStartedAndNonEmptyError() + { + // Get a free port, then bind it so the service cannot bind + var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + var port = ((IPEndPoint)listener.LocalEndpoint!).Port; + listener.Stop(); + + using var occupyingClient = new UdpClient(port); + var address = $"udp://0.0.0.0:{port}"; + + using var service = CreateService(); + var result = await service.StartListeningAsync(new List { address }); + + Assert.False(result.AnyStarted); + Assert.False(string.IsNullOrEmpty(result.ErrorMessage)); + Assert.Contains(address, result.ErrorMessage); + } + + [Fact] + public async Task StartListeningAsync_ValidPort_ReturnsStarted() + { + using var service = CreateService(); + // Port 0 lets the OS assign an available port + var address = "udp://0.0.0.0:0"; + var result = await service.StartListeningAsync(new List { address }); + + Assert.True(result.AnyStarted); + service.StopListening(); + } +} diff --git a/ui/Sentinel.NLogViewer.Wpf/RelayCommand.cs b/ui/Sentinel.NLogViewer.Wpf/RelayCommand.cs index 70705f5..cb74633 100644 --- a/ui/Sentinel.NLogViewer.Wpf/RelayCommand.cs +++ b/ui/Sentinel.NLogViewer.Wpf/RelayCommand.cs @@ -1,8 +1,82 @@ using System; +using System.Threading.Tasks; using System.Windows.Input; namespace Sentinel.NLogViewer.Wpf { + /// + /// A command implementation that executes an async operation and disables while running. + /// + public class AsyncRelayCommand : ICommand + { + private readonly Func _execute; + private readonly Func? _canExecute; + private bool _isRunning; + + /// + /// Initializes a new instance of the AsyncRelayCommand class. + /// + /// The async execution logic. + /// The execution status logic (optional). + public AsyncRelayCommand(Func execute, Func? canExecute = null) + { + _execute = execute ?? throw new ArgumentNullException(nameof(execute)); + _canExecute = canExecute; + } + + /// + /// Occurs when changes occur that affect whether or not the command should execute. + /// + public event EventHandler? CanExecuteChanged + { + add { CommandManager.RequerySuggested += value; } + remove { CommandManager.RequerySuggested -= value; } + } + + /// + /// Defines the method that determines whether the command can execute in its current state. + /// + public bool CanExecute(object? parameter) + { + if (_isRunning) + return false; + return _canExecute?.Invoke() ?? true; + } + + /// + /// Defines the method to be called when the command is invoked. + /// + public void Execute(object? parameter) + { + if (!CanExecute(parameter)) + return; + _ = ExecuteAsync(); + } + + private async Task ExecuteAsync() + { + _isRunning = true; + RaiseCanExecuteChanged(); + try + { + await _execute(); + } + finally + { + _isRunning = false; + RaiseCanExecuteChanged(); + } + } + + /// + /// Raises the CanExecuteChanged event. + /// + public void RaiseCanExecuteChanged() + { + CommandManager.InvalidateRequerySuggested(); + } + } + /// /// A command implementation that can always execute and delegates to an Action ///