Skip to content

Commit

Permalink
fix(rumqttc): acks with failure reason code should not be state error (
Browse files Browse the repository at this point in the history
  • Loading branch information
xiaocq2001 authored Jun 26, 2024
1 parent 15c3968 commit db1f261
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 26 deletions.
1 change: 1 addition & 0 deletions rumqttc/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
38 changes: 12 additions & 26 deletions rumqttc/src/v5/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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));
Expand All @@ -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;
Expand Down

0 comments on commit db1f261

Please sign in to comment.