Fix: Handle Exceptions in Traceroute (#1994)

* Fix: Handle Exceptions in Traceroute

* Docs: Add #1994

Source/NETworkManager.Models/Network/Traceroute.cs

@@ -1,8 +1,8 @@
-using DnsClient;
-using NETworkManager.Utilities;
+using NETworkManager.Utilities;
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Linq;
 using System.Net;
 using System.Net.NetworkInformation;
 using System.Threading;
@@ -13,11 +13,7 @@ namespace NETworkManager.Models.Network
     public class Traceroute
     {
         #region Variables
-        public int Timeout = 4000;
-        public byte[] Buffer = new byte[32];
-        public int MaximumHops = 30;
-        public bool DontFragement = true;
-        public bool ResolveHostname = true;
+        private readonly TracerouteOptions _options;
         #endregion
 
         #region Events
@@ -39,96 +35,124 @@ protected virtual void OnMaximumHopsReached(MaximumHopsReachedArgs e)
             MaximumHopsReached?.Invoke(this, e);
         }
 
+        public event EventHandler<TracerouteErrorArgs> TraceError;
+        protected virtual void OnTraceError(TracerouteErrorArgs e)
+        {
+            TraceError?.Invoke(this, e);
+        }
+
         public event EventHandler UserHasCanceled;
         protected virtual void OnUserHasCanceled()
         {
             UserHasCanceled?.Invoke(this, EventArgs.Empty);
         }
         #endregion
 
+        #region Constructor
+        public Traceroute(TracerouteOptions options)
+        {
+            _options = options;
+        }
+        #endregion
+
         #region Methods
         public void TraceAsync(IPAddress ipAddress, CancellationToken cancellationToken)
         {
             Task.Run(async () =>
             {
-                for (var i = 1; i < MaximumHops + 1; i++)
+                try
                 {
-                    var tasks = new List<Task<Tuple<PingReply, long>>>();
-
-                    for (var y = 0; y < 3; y++)
+                    for (var i = 1; i < _options.MaximumHops + 1; i++)
                     {
-                        var i1 = i;
+                        var tasks = new List<Task<Tuple<PingReply, long>>>();
 
-                        tasks.Add(Task.Run(() =>
+                        // Send 3 pings
+                        for (var y = 0; y < 3; y++)
                         {
-                            var stopwatch = new Stopwatch();
-
-                            PingReply pingReply;
+                            var i1 = i;
 
-                            using (var ping = new System.Net.NetworkInformation.Ping())
+                            tasks.Add(Task.Run(() =>
                             {
-                                stopwatch.Start();
+                                var stopwatch = new Stopwatch();
 
-                                pingReply = ping.Send(ipAddress, Timeout, Buffer, new PingOptions { Ttl = i1, DontFragment = DontFragement });
+                                PingReply pingReply;
 
-                                stopwatch.Stop();
-                            }
+                                using (var ping = new System.Net.NetworkInformation.Ping())
+                                {
+                                    stopwatch.Start();
 
-                            return Tuple.Create(pingReply, stopwatch.ElapsedMilliseconds);
-                        }, cancellationToken));
-                    }
+                                    pingReply = ping.Send(ipAddress, _options.Timeout, _options.Buffer, new PingOptions { Ttl = i1, DontFragment = _options.DontFragment });
 
-                    Task.WaitAll(tasks.ToArray());
+                                    stopwatch.Stop();
+                                }
 
-                    // Check results -> Get IP on success or TTL expired
-                    IPAddress ipAddressHop = null;
+                                return Tuple.Create(pingReply, stopwatch.ElapsedMilliseconds);
+                            }, cancellationToken));
+                        }
 
-                    foreach (var task in tasks)
-                    {
-                        if (task.Result.Item1.Status == IPStatus.TimedOut)
-                            continue;
-                                                
-                        if (task.Result.Item1.Status == IPStatus.TtlExpired || task.Result.Item1.Status == IPStatus.Success)
+                        try
                         {
-                            ipAddressHop = task.Result.Item1.Address;
-                            break;
-                        }                          
-                    }
-                    
-                    // Resolve Hostname
-                    var hostname = string.Empty;
+                            Task.WaitAll(tasks.ToArray());
+                        }
+                        catch (AggregateException ex)
+                        {
+                            // Remove duplicate messages
+                            OnTraceError(new TracerouteErrorArgs(string.Join(", ", ex.Flatten().InnerExceptions.Select(s => s.Message).Distinct())));
+                            return;
+                        }
 
-                    if (ResolveHostname && ipAddressHop != null)
-                    {
-                        var dnsResult = await DNSClient.GetInstance().ResolvePtrAsync(ipAddressHop);
+                        // Check results -> Get IP on success or TTL expired
+                        IPAddress ipAddressHop = null;
 
-                        if (!dnsResult.HasError)
-                            hostname = dnsResult.Value;
-                    }
+                        foreach (var task in tasks)
+                        {
+                            if (task.Result.Item1.Status == IPStatus.TimedOut)
+                                continue;
 
-                    OnHopReceived(new TracerouteHopReceivedArgs(i, tasks[0].Result.Item2, tasks[1].Result.Item2, tasks[2].Result.Item2, ipAddressHop, hostname, tasks[0].Result.Item1.Status, tasks[1].Result.Item1.Status, tasks[2].Result.Item1.Status));
+                            if (task.Result.Item1.Status == IPStatus.TtlExpired || task.Result.Item1.Status == IPStatus.Success)
+                            {
+                                ipAddressHop = task.Result.Item1.Address;
+                                break;
+                            }
+                        }
 
-                    // Check if finished
-                    if (ipAddressHop != null && ipAddress.ToString() == ipAddressHop.ToString())
-                    {
-                        OnTraceComplete();
-                        return;
-                    }
+                        // Resolve Hostname
+                        var hostname = string.Empty;
 
-                    // Check for cancel
-                    if (!cancellationToken.IsCancellationRequested)
-                        continue;
+                        if (ipAddressHop != null && _options.ResolveHostname)
+                        {
+                            var dnsResult = await DNSClient.GetInstance().ResolvePtrAsync(ipAddressHop);
 
-                    OnUserHasCanceled();
+                            if (!dnsResult.HasError)
+                                hostname = dnsResult.Value;
+                        }
 
-                    return;
-                }
+                        OnHopReceived(new TracerouteHopReceivedArgs(i, tasks[0].Result.Item2, tasks[1].Result.Item2, tasks[2].Result.Item2, ipAddressHop, hostname, tasks[0].Result.Item1.Status, tasks[1].Result.Item1.Status, tasks[2].Result.Item1.Status));
 
-                // Max hops reached...
-                OnMaximumHopsReached(new MaximumHopsReachedArgs(MaximumHops));
+                        // Check if finished
+                        if (ipAddressHop != null && ipAddress.ToString() == ipAddressHop.ToString())
+                        {
+                            OnTraceComplete();
+                            return;
+                        }
 
+                        // Check for cancel
+                        if (!cancellationToken.IsCancellationRequested)
+                            continue;
+
+                        OnUserHasCanceled();
+                        return;
+                    }
+
+                    // Max hops reached...
+                    OnMaximumHopsReached(new MaximumHopsReachedArgs(_options.MaximumHops));
+                }
+                catch (Exception ex)
+                {
+                    OnTraceError(new TracerouteErrorArgs(ex.Message));
+                }
             }, cancellationToken);
         }
         #endregion
     }
-}
+}

Source/NETworkManager.Models/Network/TracerouteErrorArgs.cs

@@ -0,0 +1,17 @@
+namespace NETworkManager.Models.Network
+{
+    public class TracerouteErrorArgs : System.EventArgs
+    {
+        public string ErrorMessage { get; set; }
+
+        public TracerouteErrorArgs()
+        {
+
+        }
+
+        public TracerouteErrorArgs(string errorMessage)
+        {
+            ErrorMessage = errorMessage;
+        }
+    }
+}

Source/NETworkManager.Models/Network/TracerouteOptions.cs

@@ -0,0 +1,58 @@
+namespace NETworkManager.Models.Network;
+
+/// <summary>
+/// Class contains the options for the traceroute.
+/// </summary>
+public class TracerouteOptions
+{
+    /// <summary>
+    /// Timeout in milliseconds after which a ping is considered lost.
+    /// </summary>
+    public int Timeout { get; set; }
+
+    /// <summary>
+    /// Size of the buffer used in the ping in bytes.    
+    /// </summary>
+    public byte[] Buffer { get; set; }
+
+    /// <summary>
+    /// Maximum number of hops between the local and remote computer.
+    /// </summary>
+    public int MaximumHops { get; set; }
+
+    /// <summary>
+    /// Do not fragment the ping packet.    
+    /// </summary>
+    public bool DontFragment { get; set; }
+
+    /// <summary>
+    /// Resolve the hostname for an IP address.
+    /// </summary>
+    public bool ResolveHostname { get; set; }
+
+    /// <summary>
+    /// Create an instance of <see cref="TracerouteOptions"/>.
+    /// </summary>
+    public TracerouteOptions()
+    {
+
+    }
+
+    /// <summary>
+    /// Create an instance of <see cref="TracerouteOptions"/> with parameters.
+    /// </summary>
+    /// <param name="timeout">Timeout in milliseconds after which a ping is considered lost.</param>
+    /// <param name="buffer">Size of the buffer used in the ping in bytes.</param>
+    /// <param name="maximumHops">Maximum number of hops between the local and remote computer.</param>
+    /// <param name="dontFragment">Do not fragment the ping packet.</param>
+    /// <param name="resolveHostname">Resolve the hostname for an IP address.</param>
+    public TracerouteOptions(int timeout, byte[] buffer, int maximumHops, bool dontFragment, bool resolveHostname)
+    {
+        Timeout = timeout;
+        Buffer = buffer;
+        MaximumHops = maximumHops;
+        DontFragment = dontFragment;
+        ResolveHostname = resolveHostname;
+    }
+}
+

Source/NETworkManager/ViewModels/TracerouteViewModel.cs

@@ -333,20 +333,21 @@ private async Task StartTrace()
 
             try
             {
-                var traceroute = new Traceroute
+                var traceroute = new Traceroute(new TracerouteOptions
                 {
                     Timeout = SettingsManager.Current.Traceroute_Timeout,
                     Buffer = new byte[SettingsManager.Current.Traceroute_Buffer],
                     MaximumHops = SettingsManager.Current.Traceroute_MaximumHops,
-                    DontFragement = true,
-                    ResolveHostname = SettingsManager.Current.Traceroute_ResolveHostname
-                };
+                    DontFragment = true,
+                    ResolveHostname = SettingsManager.Current.Traceroute_ResolveHostname                    
+                });
 
                 traceroute.HopReceived += Traceroute_HopReceived;
                 traceroute.TraceComplete += Traceroute_TraceComplete;
                 traceroute.MaximumHopsReached += Traceroute_MaximumHopsReached;
+                traceroute.TraceError += Traceroute_TraceError;
                 traceroute.UserHasCanceled += Traceroute_UserHasCanceled;
-
+                
                 traceroute.TraceAsync(ipAddress, _cancellationTokenSource.Token);
 
                 // Add the host to history
@@ -359,7 +360,13 @@ private async Task StartTrace()
                 IsRunning = false;
             }
         }
-
+
+        private void UserHasCanceled()
+        {
+            CancelTrace = false;
+            IsRunning = false;
+        }
+
         private async Task Export()
         {
             var customDialog = new CustomDialog
@@ -433,7 +440,7 @@ private void Traceroute_MaximumHopsReached(object sender, MaximumHopsReachedArgs
             IsStatusMessageDisplayed = true;
             IsRunning = false;
         }
-
+        
         private void Traceroute_UserHasCanceled(object sender, EventArgs e)
         {
             CancelTrace = false;
@@ -443,6 +450,14 @@ private void Traceroute_UserHasCanceled(object sender, EventArgs e)
             IsStatusMessageDisplayed = true;
         }
 
+        private void Traceroute_TraceError(object sender, TracerouteErrorArgs e)
+        {
+            IsRunning = false;
+
+            StatusMessage = e.ErrorMessage;
+            IsStatusMessageDisplayed = true;
+        }
+
         private void Traceroute_TraceComplete(object sender, EventArgs e)
         {
             IsRunning = false;

docs/Changelog/next-release.md

@@ -89,6 +89,8 @@ The profiles and settings are automatically migrated to the new location when th
   - Detect if the DNS result for a query is null even when the DNS server doesn't send an error code and improve the processing of the resource records answers [#1949](https://github.com/BornToBeRoot/NETworkManager/pull/1949){:target="\_blank"}
   - In some cases the DNS lookup is not completed, but the user interface indicates that it is completed [#1940](https://github.com/BornToBeRoot/NETworkManager/pull/1940){:target="\_blank"} [#1949](https://github.com/BornToBeRoot/NETworkManager/pull/1949){:target="\_blank"}
   - Detect if a dns server profile with this name already exists [#1960](https://github.com/BornToBeRoot/NETworkManager/pull/1960){:target="\_blank"}
+- **Traceroute**
+  - Don't block the UI if an Exception occurs and show an error message [#1994](https://github.com/BornToBeRoot/NETworkManager/pull/1994){:target="\_blank"}
 - **AWS Session Manager**
   - Use UTF-8 encoding for embedded PowerShell console window [#1925](https://github.com/BornToBeRoot/NETworkManager/pull/1925){:target="\_blank"}
 - **Discovery Protocol**