From 6158c9bd583f46ba5aee806c97d8d19e2ad6e376 Mon Sep 17 00:00:00 2001
From: Peter Giacomo Lombardo <peter.lombardo@hivemq.com>
Date: Mon, 6 May 2024 10:49:14 +0200
Subject: [PATCH] Better Log Messages (#152)

---
 Source/HiveMQtt/Client/HiveMQClient.cs                |  4 ++--
 Source/HiveMQtt/Client/HiveMQClientOptionsBuilder.cs  |  4 ++--
 .../HiveMQtt/Client/HiveMQClientTrafficProcessor.cs   | 11 +++++++----
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/Source/HiveMQtt/Client/HiveMQClient.cs b/Source/HiveMQtt/Client/HiveMQClient.cs
index c8afdc4e..f3ef97dc 100644
--- a/Source/HiveMQtt/Client/HiveMQClient.cs
+++ b/Source/HiveMQtt/Client/HiveMQClient.cs
@@ -137,7 +137,7 @@ public async Task<bool> DisconnectAsync(DisconnectOptions? options = null)
     {
         if (this.ConnectState != ConnectState.Connected)
         {
-            Logger.Warn("DisconnectAsync: Client is not connected.");
+            Logger.Warn("DisconnectAsync called but this client is not connected.  State is ${this.ConnectState}.");
             return false;
         }
 
@@ -343,7 +343,7 @@ public async Task<SubscribeResult> SubscribeAsync(SubscribeOptions options)
         }
         catch (TimeoutException)
         {
-            Logger.Error("Subscribe timeout.  No response received in time.");
+            Logger.Error("Subscribe timeout.  No SUBACK response received in time.");
             throw;
         }
         finally
diff --git a/Source/HiveMQtt/Client/HiveMQClientOptionsBuilder.cs b/Source/HiveMQtt/Client/HiveMQClientOptionsBuilder.cs
index 76731bf2..7f6878bb 100644
--- a/Source/HiveMQtt/Client/HiveMQClientOptionsBuilder.cs
+++ b/Source/HiveMQtt/Client/HiveMQClientOptionsBuilder.cs
@@ -181,12 +181,12 @@ public HiveMQClientOptionsBuilder WithClientCertificate(string clientCertificate
             }
             catch (UnauthorizedAccessException)
             {
-                Logger.Error("File exists but is not readable due to access permissions.");
+                Logger.Error("WithClientCertificate: File exists but is not readable due to access permissions.");
                 throw;
             }
             catch (IOException)
             {
-                Logger.Error("An I/O error occurred while trying to read the file.");
+                Logger.Error("WithClientCertificate: An I/O error occurred while trying to read the file.");
                 throw;
             }
         }
diff --git a/Source/HiveMQtt/Client/HiveMQClientTrafficProcessor.cs b/Source/HiveMQtt/Client/HiveMQClientTrafficProcessor.cs
index 19a8625c..6319176d 100644
--- a/Source/HiveMQtt/Client/HiveMQClientTrafficProcessor.cs
+++ b/Source/HiveMQtt/Client/HiveMQClientTrafficProcessor.cs
@@ -146,7 +146,7 @@ private Task<bool> ConnectionWriterAsync(CancellationToken cancellationToken) =>
                             // QoS > 0 - Add to transaction queue
                             if (this.transactionQueue.TryAdd(publishPacket.PacketIdentifier, new List<ControlPacket> { publishPacket }) == false)
                             {
-                                Logger.Warn("Duplicate packet ID detected.");
+                                Logger.Warn($"Duplicate packet ID detected {publishPacket.PacketIdentifier} while queueing to transaction queue for an outgoing QoS {publishPacket.Message.QoS} publish .");
                                 continue;
                             }
                         }
@@ -334,8 +334,8 @@ private Task<bool> ReceivedPacketsHandlerAsync(CancellationToken cancellationTok
                 {
                     if (packet.PacketSize > this.Options.ClientMaximumPacketSize)
                     {
-                        Logger.Warn($"Packet size {packet.PacketSize} exceeds maximum packet size {this.Options.ClientMaximumPacketSize}.  Disconnecting.");
-                        Logger.Debug($"{this.Options.ClientId}-(RPH)- Packet size {packet.PacketSize} exceeds maximum packet size {this.Options.ClientMaximumPacketSize}.  Disconnecting.");
+                        Logger.Warn($"Received packet size {packet.PacketSize} exceeds maximum packet size {this.Options.ClientMaximumPacketSize}.  Disconnecting.");
+                        Logger.Debug($"{this.Options.ClientId}-(RPH)- Received packet size {packet.PacketSize} exceeds maximum packet size {this.Options.ClientMaximumPacketSize}.  Disconnecting.");
 
                         var opts = new DisconnectOptions
                         {
@@ -411,6 +411,9 @@ internal void HandleIncomingPublishPacket(PublishPacket publishPacket)
         {
             // We've received a QoS 1 publish.  Send a PubAck.
             var pubAckResponse = new PubAckPacket(publishPacket.PacketIdentifier, PubAckReasonCode.Success);
+
+            // FIXME We should wait until puback is sent before launching event
+            // FIXME Check DUP flag setting
             this.SendQueue.Add(pubAckResponse);
         }
         else if (publishPacket.Message.QoS is MQTT5.Types.QualityOfService.ExactlyOnceDelivery)
@@ -421,7 +424,7 @@ internal void HandleIncomingPublishPacket(PublishPacket publishPacket)
 
             if (this.transactionQueue.TryAdd(publishPacket.PacketIdentifier, publishQoS2Chain) == false)
             {
-                Logger.Warn("QoS2: Duplicate packet ID detected.");
+                Logger.Warn($"Duplicate packet ID detected {publishPacket.PacketIdentifier} while queueing to transaction queue for an incoming QoS {publishPacket.Message.QoS} publish .");
                 pubRecResponse.ReasonCode = PubRecReasonCode.PacketIdentifierInUse;
             }