From db1f261dd5cd6c69bbfd1058ba69ea8ef5f4fc38 Mon Sep 17 00:00:00 2001
From: CQ Xiao <53201544+xiaocq2001@users.noreply.github.com>
Date: Wed, 26 Jun 2024 15:27:29 +0800
Subject: [PATCH] fix(rumqttc): acks with failure reason code should not be
 state error (#877)

---
 rumqttc/CHANGELOG.md    |  1 +
 rumqttc/src/v5/state.rs | 38 ++++++++++++--------------------------
 2 files changed, 13 insertions(+), 26 deletions(-)

diff --git a/rumqttc/CHANGELOG.md b/rumqttc/CHANGELOG.md
index 34f5438cd..c8c8716a3 100644
--- a/rumqttc/CHANGELOG.md
+++ b/rumqttc/CHANGELOG.md
@@ -36,6 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * Ordering of `State.events` related to `QoS > 0` publishes
 * Filter PUBACK in pending save requests to fix unexpected PUBACK sent to reconnected broker.
 * Resume session only if broker sends `CONNACK` with `session_present == 1`.
+* Remove v5 PubAck/PubRec/PubRel/PubComp/Sub/Unsub failures from `StateError` and log warnings on these failures.
 
 ### Security
 
diff --git a/rumqttc/src/v5/state.rs b/rumqttc/src/v5/state.rs
index 6f37a4719..0f08a33b8 100644
--- a/rumqttc/src/v5/state.rs
+++ b/rumqttc/src/v5/state.rs
@@ -51,18 +51,6 @@ pub enum StateError {
         reason_code: DisconnectReasonCode,
         reason_string: Option<String>,
     },
-    #[error("Unsubscribe failed with reason '{reason:?}' ")]
-    UnsubFail { reason: UnsubAckReason },
-    #[error("Subscribe failed with reason '{reason:?}' ")]
-    SubFail { reason: SubscribeReasonCode },
-    #[error("Publish acknowledgement failed with reason '{reason:?}' ")]
-    PubAckFail { reason: PubAckReason },
-    #[error("Publish receive failed with reason '{reason:?}' ")]
-    PubRecFail { reason: PubRecReason },
-    #[error("Publish release failed with reason '{reason:?}' ")]
-    PubRelFail { reason: PubRelReason },
-    #[error("Publish completion failed with reason '{reason:?}' ")]
-    PubCompFail { reason: PubCompReason },
     #[error("Connection failed with reason '{reason:?}' ")]
     ConnFail { reason: ConnectReturnCode },
     #[error("Connection closed by peer abruptly")]
@@ -252,7 +240,9 @@ impl MqttState {
                 SubscribeReasonCode::Success(qos) => {
                     debug!("SubAck Pkid = {:?}, QoS = {:?}", suback.pkid, qos);
                 }
-                _ => return Err(StateError::SubFail { reason: *reason }),
+                _ => {
+                    warn!("SubAck Pkid = {:?}, Reason = {:?}", suback.pkid, reason);
+                },
             }
         }
         Ok(None)
@@ -264,7 +254,7 @@ impl MqttState {
     ) -> Result<Option<Packet>, StateError> {
         for reason in unsuback.reasons.iter() {
             if reason != &UnsubAckReason::Success {
-                return Err(StateError::UnsubFail { reason: *reason });
+                warn!("UnsubAck Pkid = {:?}, Reason = {:?}", unsuback.pkid, reason);
             }
         }
         Ok(None)
@@ -374,9 +364,8 @@ impl MqttState {
         if puback.reason != PubAckReason::Success
             && puback.reason != PubAckReason::NoMatchingSubscribers
         {
-            return Err(StateError::PubAckFail {
-                reason: puback.reason,
-            });
+            warn!("PubAck Pkid = {:?}, reason: {:?}", puback.pkid, puback.reason);
+            return Ok(None);
         }
 
         if let Some(publish) = self.check_collision(puback.pkid) {
@@ -408,9 +397,8 @@ impl MqttState {
         if pubrec.reason != PubRecReason::Success
             && pubrec.reason != PubRecReason::NoMatchingSubscribers
         {
-            return Err(StateError::PubRecFail {
-                reason: pubrec.reason,
-            });
+            warn!("PubRec Pkid = {:?}, reason: {:?}", pubrec.pkid, pubrec.reason);
+            return Ok(None);
         }
 
         // NOTE: Inflight - 1 for qos2 in comp
@@ -429,9 +417,8 @@ impl MqttState {
         self.incoming_pub.set(pubrel.pkid as usize, false);
 
         if pubrel.reason != PubRelReason::Success {
-            return Err(StateError::PubRelFail {
-                reason: pubrel.reason,
-            });
+            warn!("PubRel Pkid = {:?}, reason: {:?}", pubrel.pkid, pubrel.reason);
+            return Ok(None);
         }
 
         let event = Event::Outgoing(Outgoing::PubComp(pubrel.pkid));
@@ -457,9 +444,8 @@ impl MqttState {
         self.outgoing_rel.set(pubcomp.pkid as usize, false);
 
         if pubcomp.reason != PubCompReason::Success {
-            return Err(StateError::PubCompFail {
-                reason: pubcomp.reason,
-            });
+            warn!("PubComp Pkid = {:?}, reason: {:?}", pubcomp.pkid, pubcomp.reason);
+            return Ok(None);
         }
 
         self.inflight -= 1;