Address PR comments

src/app/CommandHandler.cpp

@@ -780,8 +780,9 @@ CHIP_ERROR CommandHandler::RollbackResponse()
 {
     VerifyOrReturnError(mRollbackBackupValid, CHIP_ERROR_INCORRECT_STATE);
     VerifyOrReturnError(mState == State::Preparing || mState == State::AddingCommand, CHIP_ERROR_INCORRECT_STATE);
+    // TODO(#30453): Rollback of mInvokeResponseBuilder should handle resetting
+    // InvokeResponses.
     mInvokeResponseBuilder.GetInvokeResponses().ResetError();
-    mInvokeResponseBuilder.ResetError();
     mInvokeResponseBuilder.Rollback(mBackupWriter);
     MoveToState(mBackupState);
     mRollbackBackupValid = false;
@@ -838,14 +839,17 @@ CommandHandler::Handle::Handle(CommandHandler * handle)
 CHIP_ERROR CommandHandler::FinalizeInvokeResponseMessageAndPrepareNext()
 {
     ReturnErrorOnFailure(FinalizeInvokeResponseMessage(/* aHasMoreChunks = */ true));
+    // After successfully finalizing InvokeResponseMessage, no buffer should remain
+    // allocated.
+    VerifyOrDie(!mBufferAllocated);
     CHIP_ERROR err = AllocateBuffer();
-    if (err == CHIP_NO_ERROR)
-    {
-        VerifyOrDie(mState == State::NewResponseMessage);
-    }
-    else
+    if (err != CHIP_NO_ERROR)
     {
-        mResponseSender.SetFinalStatusResponseFailure(Protocols::InteractionModel::Status::ResourceExhausted);
+        // TODO(#30453): Improve ResponseDropped calls to occur only when dropping is
+        // definitively guaranteed.
+        // Response dropping is not yet definitive as a subsequent call
+        // to AllocateBuffer might succeed.
+        mResponseSender.ResponseDropped();
     }
     return err;
 }

src/app/CommandHandler.h

@@ -360,8 +360,17 @@ class CommandHandler
     template <typename CommandData>
     CHIP_ERROR AddResponseData(const ConcreteCommandPath & aRequestCommandPath, const CommandData & aData)
     {
-        // We are extracting the things needed by the CommandData template and keeping
-        // this function as minimal as possible to compiled code size.
+        // This method, templated with CommandData, captures all the components needs
+        // from CommandData with as little code as possible. This in theory should
+        // reduce compiled code size.
+        //
+        // TODO(#30453): Verify the accuracy of the theory outlined below.
+        //
+        // Theory on code reduction: Previously, non-essential code was unnecessarily
+        // templated, leading to compilation and duplication N times. The lambda
+        // function below mitigates this issue by isolating only the code segments
+        // that genuinely require templating, thereby minimizing duplicate compiled
+        // code.
         ConcreteCommandPath responsePath = { aRequestCommandPath.mEndpointId, aRequestCommandPath.mClusterId,
                                              CommandData::GetCommandId() };
         auto encodeCommandDataClosure    = [&](TLV::TLVWriter & writer) -> CHIP_ERROR {
@@ -563,8 +572,7 @@ class CommandHandler
      *
      * @param [in] aRequestCommandPath the concrete path of the command we are
      *             responding to.
-     * @param [in] aResponseCommandPath the concrete path of the command we are
-     *             responding to.
+     * @param [in] aResponseCommandPath the concrete command response path.
      * @param [in] encodeCommandDataFunction A lambda function responsible for
      *             encoding the CommandData field.
      */

src/app/CommandResponseSender.cpp

@@ -105,9 +105,9 @@ CHIP_ERROR CommandResponseSender::SendCommandResponse()
     VerifyOrReturnError(HasMoreToSend(), CHIP_ERROR_INCORRECT_STATE);
     if (mChunks.IsNull())
     {
-        VerifyOrReturnError(mFinalFailureStatus.HasValue(), CHIP_ERROR_INCORRECT_STATE);
-        SendStatusResponse(mFinalFailureStatus.Value());
-        mFinalFailureStatus.ClearValue();
+        VerifyOrReturnError(mReportResponseDropped, CHIP_ERROR_INCORRECT_STATE);
+        SendStatusResponse(Status::ResourceExhausted);
+        mReportResponseDropped = false;
         return CHIP_NO_ERROR;
     }
     System::PacketBufferHandle commandResponsePayload = mChunks.PopHead();

src/app/CommandResponseSender.h

@@ -124,27 +124,9 @@ class CommandResponseSender : public Messaging::ExchangeDelegate
     }
 
     /**
-     * @brief Queues a single, non-path-specific, error to be sent after all queued InvokeRequestMessages.
-     *
-     * Sends a single non-path-specific error to the client following the transmission of all queued
-     * InvokeRequestMessages.
-     *
-     * Behavior: If invoked multiple times, only the error set by the final call will be relayed
-     * to the client.
-     *
-     * @param aStatus InteractionModel Status. Cannot be success status.
-     * @return CHIP_NO_ERROR Successfully set IM failure status to send as final StatusResponse.
-     * @return CHIP_ERROR_INVALID_ARGUMENT provided with status that is success.
-     * @return CHIP_ERROR_INCORRECT_STATE
+     * @brief Called to indicate that response was dropped
      */
-    CHIP_ERROR SetFinalStatusResponseFailure(Protocols::InteractionModel::Status aStatus)
-    {
-        VerifyOrReturnError(aStatus != Protocols::InteractionModel::Status::Success, CHIP_ERROR_INVALID_ARGUMENT);
-        VerifyOrReturnError(mState != State::AllInvokeResponsesSent, CHIP_ERROR_INCORRECT_STATE);
-        VerifyOrReturnError(!mChunks.IsNull(), CHIP_ERROR_INCORRECT_STATE);
-        mFinalFailureStatus.SetValue(aStatus);
-        return CHIP_NO_ERROR;
-    }
+    void ResponseDropped() { mReportResponseDropped = true; }
 
     /**
      * @brief Registers a callback to be invoked when CommandResponseSender has finished sending responses.
@@ -167,19 +149,18 @@ class CommandResponseSender : public Messaging::ExchangeDelegate
     const char * GetStateStr() const;
 
     CHIP_ERROR SendCommandResponse();
-    bool HasMoreToSend() { return !mChunks.IsNull() || mFinalFailureStatus.HasValue(); }
+    bool HasMoreToSend() { return !mChunks.IsNull() || mReportResponseDropped; }
     void Close();
 
     // A list of InvokeResponseMessages to be sent out by CommandResponseSender.
     System::PacketBufferHandle mChunks;
 
-    // When final status is set, we will send out this status as the final message in the interaction.
-    Optional<Protocols::InteractionModel::Status> mFinalFailureStatus;
     chip::Callback::Callback<OnResponseSenderDone> * mResponseSenderDoneCallback = nullptr;
     Messaging::ExchangeHolder mExchangeCtx;
     State mState = State::ReadyForInvokeResponses;
 
     bool mCloseCalled = false;
+    bool mReportResponseDropped = false;
 };
 
 } // namespace app