Implement error handling and notification

ntfysh_client/MainForm.cs

@@ -19,10 +19,29 @@ namespace ntfysh_client
         {
             _notificationListener = notificationListener;
             _notificationListener.OnNotificationReceive += OnNotificationReceive;
+            _notificationListener.OnConnectionMultiAttemptFailure += OnConnectionMultiAttemptFailure;
+            _notificationListener.OnConnectionCredentialsFailure += OnConnectionCredentialsFailure;
             
             InitializeComponent();
         }
 
+        private void OnNotificationReceive(object sender, NotificationReceiveEventArgs e)
+        {
+            notifyIcon.ShowBalloonTip(3000, e.Title, e.Message, ToolTipIcon.Info);
+        }
+
+        private void OnConnectionMultiAttemptFailure(NotificationListener sender, SubscribedTopic topic)
+        {
+            MessageBox.Show($"Connecting to topic ID '{topic.TopicId}' on server '{topic.ServerUrl}' failed after multiple attempts.\n\nThis topic ID will be ignored and you will not receive notifications for it until you restart the application.", "Connection Failure", MessageBoxButtons.OK, MessageBoxIcon.Error);
+        }
+        
+        private void OnConnectionCredentialsFailure(NotificationListener sender, SubscribedTopic topic)
+        {
+            string reason = string.IsNullOrWhiteSpace(topic.Username) ? "credentials are required but were not provided" : "the entered credentials are incorrect";
+            
+            MessageBox.Show($"Connecting to topic ID '{topic.TopicId}' on server '{topic.ServerUrl}' failed because {reason}.\n\nThis topic ID will be ignored and you will not receive notifications for it until you correct the credentials.", "Connection Authentication Failure", MessageBoxButtons.OK, MessageBoxIcon.Error);
+        }
+
         private void MainForm_Load(object sender, EventArgs e) => LoadTopics();
 
         private void subscribeNewTopic_Click(object sender, EventArgs e)
@@ -137,7 +156,7 @@ namespace ntfysh_client
                 }
 
                 //Assemble new format
-                List<SubscribedTopic> newTopics = legacyTopics.Select(lt => new SubscribedTopic(lt, "https://ntfy.sh", null, null, null, null)).ToList();
+                List<SubscribedTopic> newTopics = legacyTopics.Select(lt => new SubscribedTopic(lt, "https://ntfy.sh", null, null)).ToList();
 
                 string newFormatSerialised = JsonConvert.SerializeObject(newTopics, Formatting.Indented);
                 
@@ -196,11 +215,6 @@ namespace ntfysh_client
             }
         }
 
-        private void OnNotificationReceive(object sender, NotificationReceiveEventArgs e)
-        {
-            notifyIcon.ShowBalloonTip(3000, e.Title, e.Message, ToolTipIcon.Info);
-        }
-
         private void MainForm_FormClosed(object sender, FormClosedEventArgs e)
         {
             notifyIcon.Dispose();

ntfysh_client/NotificationListener.cs

@@ -15,33 +15,52 @@ using System.Web;
 
 namespace ntfysh_client
 {
-    public class NotificationListener : IDisposable
+    public class NotificationListener
     {
-        private readonly HttpClient _httpClient = new();
-        private bool _isDisposed;
-
         public readonly Dictionary<string, SubscribedTopic?> SubscribedTopicsByUnique = new();
 
-        public delegate void NotificationReceiveHandler(object sender, NotificationReceiveEventArgs e);
+        public delegate void NotificationReceiveHandler(NotificationListener sender, NotificationReceiveEventArgs e);
         public event NotificationReceiveHandler? OnNotificationReceive;
         
+        public delegate void ConnectionErrorHandler(NotificationListener sender, SubscribedTopic topic);
+        public event ConnectionErrorHandler? OnConnectionMultiAttemptFailure;
+        public event ConnectionErrorHandler? OnConnectionCredentialsFailure;
+
         public NotificationListener()
         {
-            _httpClient.Timeout = TimeSpan.FromMilliseconds(Timeout.Infinite);
             ServicePointManager.DefaultConnectionLimit = 100;
         }
 
-        private async Task ListenToTopicWithHttpLongJsonAsync(HttpRequestMessage message, CancellationToken cancellationToken)
+        private async Task ListenToTopicWithHttpLongJsonAsync(HttpRequestMessage message, CancellationToken cancellationToken, SubscribedTopic topic)
         {
-            if (_isDisposed) throw new ObjectDisposedException(nameof(NotificationListener));
+            int connectionAttempts = 0;
             
             while (!cancellationToken.IsCancellationRequested)
             {
-                using HttpResponseMessage response = await _httpClient.SendAsync(message, HttpCompletionOption.ResponseHeadersRead, cancellationToken);
+                //See if we have exceeded maximum attempts
-                await using Stream body = await response.Content.ReadAsStreamAsync(cancellationToken);
+                if (connectionAttempts >= 10)
+                {
+                    //10 connection failures (1 initial + 9 reattempts)! Do not retry
+                    OnConnectionMultiAttemptFailure?.Invoke(this, topic);
+                    return;
+                }
                 
                 try
                 {
+                    //Establish connection
+                    using HttpClient client = new();
+                    client.Timeout = TimeSpan.FromMilliseconds(Timeout.Infinite); //This will not prevent us from failing to connect, luckily
+                    
+                    using HttpResponseMessage response = await client.SendAsync(message, HttpCompletionOption.ResponseHeadersRead, cancellationToken);
+                    await using Stream body = await response.Content.ReadAsStreamAsync(cancellationToken);
+                    
+                    //Ensure successful connect
+                    response.EnsureSuccessStatusCode();
+
+                    //Reset connection attempts after a successful connect
+                    connectionAttempts = 0;
+
+                    //Begin listening
                     StringBuilder mainBuffer = new();
 
                     while (!cancellationToken.IsCancellationRequested)
@@ -73,32 +92,82 @@ namespace ntfysh_client
                         mainBuffer.Append(partialLine);
                     }
                 }
-                catch (Exception ex)
+                catch (HttpRequestException hre)
                 {
+                    if (hre.StatusCode is HttpStatusCode.Unauthorized or HttpStatusCode.Forbidden)
+                    {
+                        //Our credentials either aren't present when they need to be or are invalid
+                        
+                        //Credential Failure! Do not retry
+                        OnConnectionCredentialsFailure?.Invoke(this, topic);
+                        return;
+                    }
+
                     #if DEBUG
-                        Debug.WriteLine(ex);
+                        Debug.WriteLine(hre);
                     #endif
                     
-                    //Fall back to the outer loop to restart the listen, or cancel if requested
+                    //We will not hit the finally block which will increment the connection failure counter and attempt a reconnect if applicable
+                }
+                catch (Exception e)
+                {
+                    #if DEBUG
+                        Debug.WriteLine(e);
+                    #endif
+
+                    //We will not hit the finally block which will increment the connection failure counter and attempt a reconnect if applicable
+                }
+                finally
+                {
+                    //We land here if we fail to connect or our connection gets closed (and if we are canceeling, but that gets ignored)
+                    
+                    if (!cancellationToken.IsCancellationRequested)
+                    {
+                        //Not cancelling, legitimate connection failure or termination
+
+                        if (connectionAttempts != 0)
+                        {
+                            //On our first reconnect attempt, try instantly. On consecutive, wait 3 seconds before each attempt
+                            await Task.Delay(TimeSpan.FromSeconds(3), cancellationToken);
+                        }
+
+                        //Increment attempts
+                        connectionAttempts++;
+                        
+                        //Proceed to reattempt
+                    }
                 }
             }
         }
         
-        private async Task ListenToTopicWithWebsocketAsync(Uri uri, NetworkCredential credentials, CancellationToken cancellationToken)
+        private async Task ListenToTopicWithWebsocketAsync(Uri uri, NetworkCredential credentials, CancellationToken cancellationToken, SubscribedTopic topic)
         {
-            if (_isDisposed) throw new ObjectDisposedException(nameof(NotificationListener));
+            int connectionAttempts = 0;
             
             while (!cancellationToken.IsCancellationRequested)
             {
-                using ClientWebSocket socket = new();
+                //See if we have exceeded maximum attempts
-                socket.Options.Credentials = credentials;
+                if (connectionAttempts >= 10)
+                {
+                    //10 connection failures (1 initial + 9 reattempts)! Do not retry
+                    OnConnectionMultiAttemptFailure?.Invoke(this, topic);
+                    return;
+                }
 
                 try
                 {
-                    StringBuilder mainBuffer = new();
+                    //Establish connection
+                    using ClientWebSocket socket = new();
+                    socket.Options.Credentials = credentials;
                     
                     await socket.ConnectAsync(uri, cancellationToken);
                     
+                    //Reset connection attempts after a successful connect
+                    connectionAttempts = 0;
+                    
+                    //Begin listening
+                    StringBuilder mainBuffer = new();
+
                     while (!cancellationToken.IsCancellationRequested)
                     {
                         //Read as much as possible
@@ -126,13 +195,50 @@ namespace ntfysh_client
                         mainBuffer.Append(partialLine);
                     }
                 }
-                catch (Exception ex)
+                catch (WebSocketException wse)
                 {
+                    if (wse.WebSocketErrorCode is WebSocketError.NotAWebSocket)
+                    {
+                        //We haven't achieved a connection with a websocket. TODO Seems ntfy doesn't report unauthorised properly, and responds 200
+                        
+                        //Credential Failure! Do not retry
+                        OnConnectionCredentialsFailure?.Invoke(this, topic);
+                        return;
+                    }
+                    
                     #if DEBUG
-                        Debug.WriteLine(ex);
+                        Debug.WriteLine(wse);
                     #endif
                     
-                    //Fall back to the outer loop to restart the listen, or cancel if requested
+                    //We will not hit the finally block which will increment the connection failure counter and attempt a reconnect if applicable
+                }
+                catch (Exception e)
+                {
+                    #if DEBUG
+                        Debug.WriteLine(e);
+                    #endif
+                    
+                    //We will not hit the finally block which will increment the connection failure counter and attempt a reconnect if applicable
+                }
+                finally
+                {
+                    //We land here if we fail to connect or our connection gets closed (and if we are canceeling, but that gets ignored)
+                    
+                    if (!cancellationToken.IsCancellationRequested)
+                    {
+                        //Not cancelling, legitimate connection failure or termination
+
+                        if (connectionAttempts != 0)
+                        {
+                            //On our first reconnect attempt, try instantly. On consecutive, wait 3 seconds before each attempt
+                            await Task.Delay(TimeSpan.FromSeconds(3), cancellationToken);
+                        }
+
+                        //Increment attempts
+                        connectionAttempts++;
+                        
+                        //Proceed to reattempt
+                    }
                 }
             }
         }
@@ -156,8 +262,6 @@ namespace ntfysh_client
 
         public void SubscribeToTopicUsingLongHttpJson(string unique, string topicId, string serverUrl, string? username, string? password)
         {
-            if (_isDisposed) throw new ObjectDisposedException(nameof(NotificationListener));
-            
             if (SubscribedTopicsByUnique.ContainsKey(unique)) throw new InvalidOperationException("A topic with this unique already exists");
             
             if (string.IsNullOrWhiteSpace(username)) username = null;
@@ -172,31 +276,35 @@ namespace ntfysh_client
                 message.Headers.Authorization = new AuthenticationHeaderValue("Basic", Convert.ToBase64String(boundCredentialsBytes));
             }
 
-            CancellationTokenSource listenCanceller = new();
+            SubscribedTopic newTopic = new(topicId, serverUrl, username, password);
-            Task listenTask = ListenToTopicWithHttpLongJsonAsync(message, listenCanceller.Token);
 
-            SubscribedTopicsByUnique.Add(unique, new SubscribedTopic(topicId, serverUrl, username, password, listenTask, listenCanceller));
+            CancellationTokenSource listenCanceller = new();
+            Task listenTask = ListenToTopicWithHttpLongJsonAsync(message, listenCanceller.Token, newTopic);
+            
+            newTopic.SetAssociatedRunner(listenTask, listenCanceller);
+
+            SubscribedTopicsByUnique.Add(unique, newTopic);
         }
         
         public void SubscribeToTopicUsingWebsocket(string unique, string topicId, string serverUrl, string? username, string? password)
         {
-            if (_isDisposed) throw new ObjectDisposedException(nameof(NotificationListener));
-            
             if (SubscribedTopicsByUnique.ContainsKey(unique)) throw new InvalidOperationException("A topic with this unique already exists");
             
             if (string.IsNullOrWhiteSpace(username)) username = null;
             if (string.IsNullOrWhiteSpace(password)) password = null;
             
-            Uri targetUri = new($"{serverUrl}/{HttpUtility.UrlEncode(topicId)}/ws");
+            SubscribedTopic newTopic = new(topicId, serverUrl, username, password);
+            
             CancellationTokenSource listenCanceller = new();
-            Task listenTask = ListenToTopicWithWebsocketAsync(targetUri, new NetworkCredential(username, password), listenCanceller.Token);
+            Task listenTask = ListenToTopicWithWebsocketAsync(new Uri($"{serverUrl}/{HttpUtility.UrlEncode(topicId)}/ws"), new NetworkCredential(username, password), listenCanceller.Token, newTopic);
-            SubscribedTopicsByUnique.Add(unique, new SubscribedTopic(topicId, serverUrl, username, password, listenTask, listenCanceller));
+            
+            newTopic.SetAssociatedRunner(listenTask, listenCanceller);
+            
+            SubscribedTopicsByUnique.Add(unique, newTopic);
         }
 
         public async Task UnsubscribeFromTopicAsync(string topicUniqueString)
         {
-            if (_isDisposed) throw new ObjectDisposedException(nameof(NotificationListener));
-                
             #if DEBUG
                 Debug.WriteLine($"Removing topic {topicUniqueString}");
             #endif
@@ -208,12 +316,12 @@ namespace ntfysh_client
             if (!SubscribedTopicsByUnique.TryGetValue(topicUniqueString, out topic!)) return;
             
             //Cancel and dispose the task runner
-            topic.RunnerCanceller.Cancel();
+            topic.RunnerCanceller?.Cancel();
 
             //Wait for the task runner to shut down
             try
             {
-                await topic.Runner;
+                if (topic.Runner is not null) await topic.Runner;
             }
             catch (Exception)
             {
@@ -221,19 +329,10 @@ namespace ntfysh_client
             }
 
             //Dispose task
-            topic.Runner.Dispose();
+            topic.Runner?.Dispose();
 
             //Remove the old topic
             SubscribedTopicsByUnique.Remove(topicUniqueString);
         }
-
-        public void Dispose()
-        {
-            if (_isDisposed) return;
-            
-            _httpClient.Dispose();
-            
-            _isDisposed = true;
-        }
     }
 }

ntfysh_client/SubscribedTopic.cs

@@ -1,4 +1,5 @@
-using System.Threading;
+using System;
+using System.Threading;
 using System.Threading.Tasks;
 using Newtonsoft.Json;
 
@@ -6,12 +7,18 @@ namespace ntfysh_client
 {
     public class SubscribedTopic
     {
-        public SubscribedTopic(string topicId, string serverUrl, string? username, string? password, Task runner, CancellationTokenSource runnerCanceller)
+        public SubscribedTopic(string topicId, string serverUrl, string? username, string? password)
         {
             TopicId = topicId;
             ServerUrl = serverUrl;
             Username = username;
             Password = password;
+        }
+
+        public void SetAssociatedRunner(Task runner, CancellationTokenSource runnerCanceller)
+        {
+            if (Runner is not null || RunnerCanceller is not null) throw new InvalidOperationException("Runner already associated");
+
             Runner = runner;
             RunnerCanceller = runnerCanceller;
         }
@@ -22,9 +29,9 @@ namespace ntfysh_client
         public string? Password { get; }
 
         [JsonIgnore]
-        public Task Runner { get; }
+        public Task? Runner { get; private set; }
         
         [JsonIgnore]
-        public CancellationTokenSource RunnerCanceller { get; }
+        public CancellationTokenSource? RunnerCanceller { get; private set; }
     }
 }