From 557278530fb0bd064b54447539e93352e0e0f7de Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 15 Sep 2023 13:07:38 -0400 Subject: [PATCH] Stop logging errors on receving a redundant standalone ack. (#29166) If we had the following message flow: 1. Send a message that is the last message on an exchange. 2. Before we get the ack retransmit the message. 3. Get acks for both messages. we would log an error on the second ack, since there was no exchagne to dispatch it to, so it would just get dropped. But dropping a standalone ack for an exchange that no longer exists is fine; in the case above the exchange no longer exists because the first ack it got allowed us to clean it up. --- src/messaging/ExchangeMgr.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/messaging/ExchangeMgr.cpp b/src/messaging/ExchangeMgr.cpp index 1ffce3e3de0316..dddc9c6fd4ce8f 100644 --- a/src/messaging/ExchangeMgr.cpp +++ b/src/messaging/ExchangeMgr.cpp @@ -300,9 +300,17 @@ void ExchangeManager::OnMessageReceived(const PacketHeader & packetHeader, const // an ack to the peer. else if (!payloadHeader.NeedsAck()) { - // Using same error message for all errors to reduce code size. - ChipLogError(ExchangeManager, "OnMessageReceived failed, err = %" CHIP_ERROR_FORMAT, - CHIP_ERROR_UNSOLICITED_MSG_NO_ORIGINATOR.Format()); + // We can easily get standalone acks here: any time we fail to get a + // timely ack for the last message in an exchange and retransmit it, + // then get acks for both the message and the retransmit, the second ack + // will end up in this block. That's not really an error condition, so + // there is no need to log an error in that case. + if (!payloadHeader.HasMessageType(Protocols::SecureChannel::MsgType::StandaloneAck)) + { + // Using same error message for all errors to reduce code size. + ChipLogError(ExchangeManager, "OnMessageReceived failed, err = %" CHIP_ERROR_FORMAT, + CHIP_ERROR_UNSOLICITED_MSG_NO_ORIGINATOR.Format()); + } return; }