Skip to content

Commit

Permalink
Stop logging errors on receving a redundant standalone ack. (#29166)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Oct 12, 2023
1 parent 6d0aaaf commit 5572785
Showing 1 changed file with 11 additions and 3 deletions.
14 changes: 11 additions & 3 deletions src/messaging/ExchangeMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down

0 comments on commit 5572785

Please sign in to comment.