Revert "Make AddStatus generally VerifyOrDie and have centralized logging (#28634)" (#28661)

bzbarsky-apple · pull[bot] · commit 1157819b846c · 2024-01-09T15:53:58.000Z
diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp
@@ -272,7 +272,7 @@ Status CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommandElem
             ChipLogDetail(DataManagement, "No command " ChipLogFormatMEI " in Cluster " ChipLogFormatMEI " on Endpoint 0x%x",
                           ChipLogValueMEI(concretePath.mCommandId), ChipLogValueMEI(concretePath.mClusterId),
                           concretePath.mEndpointId);
-            return FallibleAddStatus(concretePath, commandExists) != CHIP_NO_ERROR ? Status::Failure : Status::Success;
+            return AddStatus(concretePath, commandExists) != CHIP_NO_ERROR ? Status::Failure : Status::Success;
         }
     }
 
@@ -287,18 +287,18 @@ Status CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommandElem
         {
             if (err != CHIP_ERROR_ACCESS_DENIED)
             {
-                return FallibleAddStatus(concretePath, Status::Failure) != CHIP_NO_ERROR ? Status::Failure : Status::Success;
+                return AddStatus(concretePath, Status::Failure) != CHIP_NO_ERROR ? Status::Failure : Status::Success;
             }
             // TODO: when wildcard invokes are supported, handle them to discard rather than fail with status
-            return FallibleAddStatus(concretePath, Status::UnsupportedAccess) != CHIP_NO_ERROR ? Status::Failure : Status::Success;
+            return AddStatus(concretePath, Status::UnsupportedAccess) != CHIP_NO_ERROR ? Status::Failure : Status::Success;
         }
     }
 
     if (CommandNeedsTimedInvoke(concretePath.mClusterId, concretePath.mCommandId) && !IsTimedInvoke())
     {
         // TODO: when wildcard invokes are supported, discard a
         // wildcard-expanded path instead of returning a status.
-        return FallibleAddStatus(concretePath, Status::NeedsTimedInteraction) != CHIP_NO_ERROR ? Status::Failure : Status::Success;
+        return AddStatus(concretePath, Status::NeedsTimedInteraction) != CHIP_NO_ERROR ? Status::Failure : Status::Success;
     }
 
     if (CommandIsFabricScoped(concretePath.mClusterId, concretePath.mCommandId))
@@ -312,7 +312,7 @@ Status CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommandElem
         {
             // TODO: when wildcard invokes are supported, discard a
             // wildcard-expanded path instead of returning a status.
-            return FallibleAddStatus(concretePath, Status::UnsupportedAccess) != CHIP_NO_ERROR ? Status::Failure : Status::Success;
+            return AddStatus(concretePath, Status::UnsupportedAccess) != CHIP_NO_ERROR ? Status::Failure : Status::Success;
         }
     }
 
@@ -337,7 +337,7 @@ Status CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommandElem
 exit:
     if (err != CHIP_NO_ERROR)
     {
-        return FallibleAddStatus(concretePath, Status::InvalidCommand) != CHIP_NO_ERROR ? Status::Failure : Status::Success;
+        return AddStatus(concretePath, Status::InvalidCommand) != CHIP_NO_ERROR ? Status::Failure : Status::Success;
     }
 
     // We have handled the error status above and put the error status in response, now return success status so we can process
@@ -468,31 +468,31 @@ CHIP_ERROR CommandHandler::AddStatusInternal(const ConcreteCommandPath & aComman
     return FinishStatus();
 }
 
-void CommandHandler::AddStatus(const ConcreteCommandPath & aCommandPath, const Protocols::InteractionModel::Status aStatus,
-                               const char * context)
+CHIP_ERROR CommandHandler::AddStatus(const ConcreteCommandPath & aCommandPath, const Status aStatus)
 {
-
-    VerifyOrDie(FallibleAddStatus(aCommandPath, aStatus, context) == CHIP_NO_ERROR);
+    return AddStatusInternal(aCommandPath, StatusIB(aStatus));
 }
 
-CHIP_ERROR CommandHandler::FallibleAddStatus(const ConcreteCommandPath & path, const Protocols::InteractionModel::Status status,
-                                             const char * context)
+void CommandHandler::AddStatusAndLogIfFailure(const ConcreteCommandPath & aCommandPath, const Status aStatus, const char * aMessage)
 {
-
-    if (status != Status::Success)
+    if (aStatus != Status::Success)
     {
-        if (context == nullptr)
-        {
-            context = "no additional context";
-        }
-
         ChipLogError(DataManagement,
-                     "Endpoint=%u Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI " status " ChipLogFormatIMStatus " (%s)",
-                     path.mEndpointId, ChipLogValueMEI(path.mClusterId), ChipLogValueMEI(path.mCommandId),
-                     ChipLogValueIMStatus(status), context);
+                     "Failed to handle on Endpoint=%u Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI
+                     " with " ChipLogFormatIMStatus ": %s",
+                     aCommandPath.mEndpointId, ChipLogValueMEI(aCommandPath.mClusterId), ChipLogValueMEI(aCommandPath.mCommandId),
+                     ChipLogValueIMStatus(aStatus), aMessage);
     }
 
-    return AddStatusInternal(path, StatusIB(status));
+    CHIP_ERROR err = AddStatus(aCommandPath, aStatus);
+    if (err != CHIP_NO_ERROR)
+    {
+        ChipLogError(DataManagement,
+                     "Failed to set status on Endpoint=%u Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI
+                     ": %" CHIP_ERROR_FORMAT,
+                     aCommandPath.mEndpointId, ChipLogValueMEI(aCommandPath.mClusterId), ChipLogValueMEI(aCommandPath.mCommandId),
+                     err.Format());
+    }
 }
 
 CHIP_ERROR CommandHandler::AddClusterSpecificSuccess(const ConcreteCommandPath & aCommandPath, ClusterStatus aClusterStatus)
diff --git a/src/app/CommandHandler.h b/src/app/CommandHandler.h
@@ -170,20 +170,13 @@ class CommandHandler : public Messaging::ExchangeDelegate
      */
     void OnInvokeCommandRequest(Messaging::ExchangeContext * ec, const PayloadHeader & payloadHeader,
                                 System::PacketBufferHandle && payload, bool isTimedInvoke);
+    CHIP_ERROR AddStatus(const ConcreteCommandPath & aCommandPath, const Protocols::InteractionModel::Status aStatus);
 
-    /**
-     * Adds the given command status and returns any failures in adding statuses (e.g. out
-     * of buffer space) to the caller
-     */
-    CHIP_ERROR FallibleAddStatus(const ConcreteCommandPath & aCommandPath, const Protocols::InteractionModel::Status aStatus,
-                                 const char * context = nullptr);
-
-    /**
-     * Adds a status when the caller is unable to handle any failures. Logging is performed
-     * and failure to register the status is checked with VerifyOrDie.
-     */
-    void AddStatus(const ConcreteCommandPath & aCommandPath, const Protocols::InteractionModel::Status aStatus,
-                   const char * context = nullptr);
+    // Same as AddStatus, but logs that the command represented by aCommandPath failed with the given
+    // error status and error message, if aStatus is an error. Errors on AddStatus are just logged
+    // (since the caller likely can only log and not further add a status).
+    void AddStatusAndLogIfFailure(const ConcreteCommandPath & aCommandPath, const Protocols::InteractionModel::Status aStatus,
+                                  const char * aMessage);
 
     CHIP_ERROR AddClusterSpecificSuccess(const ConcreteCommandPath & aCommandPath, ClusterStatus aClusterStatus);
 
@@ -245,9 +238,13 @@ class CommandHandler : public Messaging::ExchangeDelegate
     template <typename CommandData>
     void AddResponse(const ConcreteCommandPath & aRequestCommandPath, const CommandData & aData)
     {
-        if (AddResponseData(aRequestCommandPath, aData) != CHIP_NO_ERROR)
+        if (CHIP_NO_ERROR != AddResponseData(aRequestCommandPath, aData))
         {
-            AddStatus(aRequestCommandPath, Protocols::InteractionModel::Status::Failure);
+            CHIP_ERROR err = AddStatus(aRequestCommandPath, Protocols::InteractionModel::Status::Failure);
+            if (err != CHIP_NO_ERROR)
+            {
+                ChipLogError(DataManagement, "Failed to encode status: %" CHIP_ERROR_FORMAT, err.Format());
+            }
         }
     }
 
diff --git a/src/app/CommandResponseHelper.h b/src/app/CommandResponseHelper.h
@@ -53,7 +53,7 @@ class CommandResponseHelper
 
     CHIP_ERROR Failure(Protocols::InteractionModel::Status aStatus)
     {
-        CHIP_ERROR err = mCommandHandler->FallibleAddStatus(mCommandPath, aStatus);
+        CHIP_ERROR err = mCommandHandler->AddStatus(mCommandPath, aStatus);
         if (err == CHIP_NO_ERROR)
         {
             mSentResponse = true;
diff --git a/src/app/clusters/barrier-control-server/barrier-control-server.cpp b/src/app/clusters/barrier-control-server/barrier-control-server.cpp
@@ -286,7 +286,10 @@ void emberAfBarrierControlClusterServerTickCallback(EndpointId endpoint)
 
 static void sendDefaultResponse(app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath, Status status)
 {
-    commandObj->AddStatus(commandPath, status);
+    if (commandObj->AddStatus(commandPath, status) != CHIP_NO_ERROR)
+    {
+        ChipLogProgress(Zcl, "Failed to send default response");
+    }
 }
 
 bool emberAfBarrierControlClusterBarrierControlGoToPercentCallback(
diff --git a/src/app/clusters/door-lock-server/door-lock-server.cpp b/src/app/clusters/door-lock-server/door-lock-server.cpp
@@ -3314,20 +3314,23 @@ bool DoorLockServer::RemoteOperationEnabled(chip::EndpointId endpointId) const
         mode != OperatingModeEnum::kPrivacy && mode != OperatingModeEnum::kNoRemoteLockUnlock;
 }
 
-void DoorLockServer::sendClusterResponse(chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,
-                                         EmberAfStatus status)
+CHIP_ERROR DoorLockServer::sendClusterResponse(chip::app::CommandHandler * commandObj,
+                                               const chip::app::ConcreteCommandPath & commandPath, EmberAfStatus status)
 {
     VerifyOrDie(nullptr != commandObj);
 
+    auto err             = CHIP_NO_ERROR;
     auto statusAsInteger = to_underlying(status);
     if (statusAsInteger == to_underlying(DlStatus::kOccupied) || statusAsInteger == to_underlying(DlStatus::kDuplicate))
     {
-        VerifyOrDie(commandObj->AddClusterSpecificFailure(commandPath, static_cast<chip::ClusterStatus>(status)) == CHIP_NO_ERROR);
+        err = commandObj->AddClusterSpecificFailure(commandPath, static_cast<chip::ClusterStatus>(status));
     }
     else
     {
-        commandObj->AddStatus(commandPath, ToInteractionModelStatus(status));
+        err = commandObj->AddStatus(commandPath, ToInteractionModelStatus(status));
     }
+
+    return err;
 }
 
 EmberAfDoorLockEndpointContext * DoorLockServer::getContext(chip::EndpointId endpointId)
diff --git a/src/app/clusters/door-lock-server/door-lock-server.h b/src/app/clusters/door-lock-server/door-lock-server.h
@@ -422,8 +422,8 @@ class DoorLockServer
 
     bool engageLockout(chip::EndpointId endpointId);
 
-    static void sendClusterResponse(chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,
-                                    EmberAfStatus status);
+    static CHIP_ERROR sendClusterResponse(chip::app::CommandHandler * commandObj,
+                                          const chip::app::ConcreteCommandPath & commandPath, EmberAfStatus status);
 
     /**
      * @brief Common handler for LockDoor, UnlockDoor, UnlockWithTimeout commands
diff --git a/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp b/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp
@@ -50,7 +50,11 @@ using Transport::Session;
     {                                                                                                                              \
         if (!::chip::ChipError::IsSuccess(expr))                                                                                   \
         {                                                                                                                          \
-            commandObj->AddStatus(commandPath, Protocols::InteractionModel::Status::code, #expr);                                  \
+            CHIP_ERROR statusErr = commandObj->AddStatus(commandPath, Protocols::InteractionModel::Status::code);                  \
+            if (statusErr != CHIP_NO_ERROR)                                                                                        \
+            {                                                                                                                      \
+                ChipLogError(Zcl, "%s: %" CHIP_ERROR_FORMAT, #expr, statusErr.Format());                                           \
+            }                                                                                                                      \
             return true;                                                                                                           \
         }                                                                                                                          \
     } while (false)
diff --git a/src/app/clusters/group-key-mgmt-server/group-key-mgmt-server.cpp b/src/app/clusters/group-key-mgmt-server/group-key-mgmt-server.cpp
@@ -414,13 +414,13 @@ bool GetProviderAndFabric(chip::app::CommandHandler * commandObj, const chip::ap
 
     if (nullptr == provider)
     {
-        commandObj->AddStatus(commandPath, Status::Failure, "Internal consistency error on provider!");
+        commandObj->AddStatusAndLogIfFailure(commandPath, Status::Failure, "Internal consistency error on provider!");
         return false;
     }
 
     if (nullptr == fabric)
     {
-        commandObj->AddStatus(commandPath, Status::Failure, "Internal consistency error on access fabric!");
+        commandObj->AddStatusAndLogIfFailure(commandPath, Status::Failure, "Internal consistency error on access fabric!");
         return false;
     }
 
@@ -460,7 +460,7 @@ bool emberAfGroupKeyManagementClusterKeySetWriteCallback(
     Status status = ValidateKeySetWriteArguments(commandData);
     if (status != Status::Success)
     {
-        commandObj->AddStatus(commandPath, status, "Failure to validate KeySet data dependencies.");
+        commandObj->AddStatusAndLogIfFailure(commandPath, status, "Failure to validate KeySet data dependencies.");
         return true;
     }
 
@@ -470,7 +470,8 @@ bool emberAfGroupKeyManagementClusterKeySetWriteCallback(
         // supported by the server, because it is ... a new value unrecognized
         // by a legacy server, then the server SHALL generate a general
         // constraint error
-        commandObj->AddStatus(commandPath, Status::ConstraintError, "Received unknown GroupKeySecurityPolicyEnum value");
+        commandObj->AddStatusAndLogIfFailure(commandPath, Status::ConstraintError,
+                                             "Received unknown GroupKeySecurityPolicyEnum value");
         return true;
     }
 
@@ -481,8 +482,8 @@ bool emberAfGroupKeyManagementClusterKeySetWriteCallback(
         // any action attempting to set CacheAndSync in the
         // GroupKeySecurityPolicy field SHALL fail with an INVALID_COMMAND
         // error.
-        commandObj->AddStatus(commandPath, Status::InvalidCommand,
-                              "Received a CacheAndSync GroupKeySecurityPolicyEnum when MCSP not supported");
+        commandObj->AddStatusAndLogIfFailure(commandPath, Status::InvalidCommand,
+                                             "Received a CacheAndSync GroupKeySecurityPolicyEnum when MCSP not supported");
         return true;
     }
 
@@ -528,7 +529,7 @@ bool emberAfGroupKeyManagementClusterKeySetWriteCallback(
     err = provider->SetKeySet(fabric->GetFabricIndex(), compressed_fabric_id, keyset);
     if (CHIP_ERROR_INVALID_LIST_LENGTH == err)
     {
-        commandObj->AddStatus(commandPath, Status::ResourceExhausted, "Not enough space left to add a new KeySet");
+        commandObj->AddStatusAndLogIfFailure(commandPath, Status::ResourceExhausted, "Not enough space left to add a new KeySet");
         return true;
     }
 
@@ -564,7 +565,7 @@ bool emberAfGroupKeyManagementClusterKeySetReadCallback(
     if (CHIP_NO_ERROR != provider->GetKeySet(fabricIndex, commandData.groupKeySetID, keyset))
     {
         // KeySet ID not found
-        commandObj->AddStatus(commandPath, Status::NotFound, "Keyset ID not found in KeySetRead");
+        commandObj->AddStatusAndLogIfFailure(commandPath, Status::NotFound, "Keyset ID not found in KeySetRead");
         return true;
     }
 
@@ -628,7 +629,8 @@ bool emberAfGroupKeyManagementClusterKeySetRemoveCallback(
     {
         // SPEC: This command SHALL fail with an INVALID_COMMAND status code back to the initiator if the GroupKeySetID being
         // removed is 0, which is the Key Set associated with the Identity Protection Key (IPK).
-        commandObj->AddStatus(commandPath, Status::InvalidCommand, "Attempted to KeySetRemove the identity protection key!");
+        commandObj->AddStatusAndLogIfFailure(commandPath, Status::InvalidCommand,
+                                             "Attempted to KeySetRemove the identity protection key!");
         return true;
     }
 
@@ -647,7 +649,7 @@ bool emberAfGroupKeyManagementClusterKeySetRemoveCallback(
     }
 
     // Send status response.
-    commandObj->AddStatus(commandPath, status, "KeySetRemove failed");
+    commandObj->AddStatusAndLogIfFailure(commandPath, status, "KeySetRemove failed");
     return true;
 }
 
@@ -699,7 +701,7 @@ bool emberAfGroupKeyManagementClusterKeySetReadAllIndicesCallback(
     auto keysIt             = provider->IterateKeySets(fabricIndex);
     if (nullptr == keysIt)
     {
-        commandObj->AddStatus(commandPath, Status::Failure, "Failed iteration of key set indices!");
+        commandObj->AddStatusAndLogIfFailure(commandPath, Status::Failure, "Failed iteration of key set indices!");
         return true;
     }
 
diff --git a/src/app/clusters/groups-server/groups-server.cpp b/src/app/clusters/groups-server/groups-server.cpp
@@ -351,7 +351,11 @@ bool emberAfGroupsClusterAddGroupIfIdentifyingCallback(app::CommandHandler * com
         status = GroupAdd(fabricIndex, endpointId, groupId, groupName);
     }
 
-    commandObj->AddStatus(commandPath, status);
+    CHIP_ERROR sendErr = commandObj->AddStatus(commandPath, status);
+    if (CHIP_NO_ERROR != sendErr)
+    {
+        ChipLogDetail(Zcl, "Groups: failed to send %s: %" CHIP_ERROR_FORMAT, "status_response", sendErr.Format());
+    }
     return true;
 }
 
diff --git a/src/app/clusters/window-covering-server/window-covering-server.cpp b/src/app/clusters/window-covering-server/window-covering-server.cpp
@@ -771,8 +771,7 @@ bool emberAfWindowCoveringClusterStopMotionCallback(app::CommandHandler * comman
         }
     }
 
-    commandObj->AddStatus(commandPath, Status::Success);
-    return true;
+    return CHIP_NO_ERROR == commandObj->AddStatus(commandPath, Status::Success);
 }
 
 /**
diff --git a/src/app/tests/TestCommandInteraction.cpp b/src/app/tests/TestCommandInteraction.cpp
@@ -1095,8 +1095,9 @@ void TestCommandInteraction::TestCommandHandlerCommandEncodeExternalFailure(nlTe
 
     err = commandHandler.AddResponseData(ConcreteCommandPath(path.mEndpointId, path.mClusterId, path.mCommandId), BadFields());
     NL_TEST_ASSERT(apSuite, err != CHIP_NO_ERROR);
-    commandHandler.AddStatus(ConcreteCommandPath(path.mEndpointId, path.mClusterId, path.mCommandId),
-                             Protocols::InteractionModel::Status::Failure);
+    err = commandHandler.AddStatus(ConcreteCommandPath(path.mEndpointId, path.mClusterId, path.mCommandId),
+                                   Protocols::InteractionModel::Status::Failure);
+    NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
     err = commandHandler.Finalize(commandPacket);
     NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
 
diff --git a/src/controller/tests/data_model/TestCommands.cpp b/src/controller/tests/data_model/TestCommands.cpp

Original file line number	Diff line number	Diff line change
`@@ -272,7 +272,7 @@ Status CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommandElem`
`272`	`272`	`ChipLogDetail(DataManagement, "No command " ChipLogFormatMEI " in Cluster " ChipLogFormatMEI " on Endpoint 0x%x",`
`273`	`273`	`ChipLogValueMEI(concretePath.mCommandId), ChipLogValueMEI(concretePath.mClusterId),`
`274`	`274`	`concretePath.mEndpointId);`
`275`		`- return FallibleAddStatus(concretePath, commandExists) != CHIP_NO_ERROR ? Status::Failure : Status::Success;`
	`275`	`+ return AddStatus(concretePath, commandExists) != CHIP_NO_ERROR ? Status::Failure : Status::Success;`
`276`	`276`	`}`
`277`	`277`	`}`
`278`	`278`
`@@ -287,18 +287,18 @@ Status CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommandElem`
`287`	`287`	`{`
`288`	`288`	`if (err != CHIP_ERROR_ACCESS_DENIED)`
`289`	`289`	`{`
`290`		`- return FallibleAddStatus(concretePath, Status::Failure) != CHIP_NO_ERROR ? Status::Failure : Status::Success;`
	`290`	`+ return AddStatus(concretePath, Status::Failure) != CHIP_NO_ERROR ? Status::Failure : Status::Success;`
`291`	`291`	`}`
`292`	`292`	`// TODO: when wildcard invokes are supported, handle them to discard rather than fail with status`
`293`		`- return FallibleAddStatus(concretePath, Status::UnsupportedAccess) != CHIP_NO_ERROR ? Status::Failure : Status::Success;`
	`293`	`+ return AddStatus(concretePath, Status::UnsupportedAccess) != CHIP_NO_ERROR ? Status::Failure : Status::Success;`
`294`	`294`	`}`
`295`	`295`	`}`
`296`	`296`
`297`	`297`	`if (CommandNeedsTimedInvoke(concretePath.mClusterId, concretePath.mCommandId) && !IsTimedInvoke())`
`298`	`298`	`{`
`299`	`299`	`// TODO: when wildcard invokes are supported, discard a`
`300`	`300`	`// wildcard-expanded path instead of returning a status.`
`301`		`- return FallibleAddStatus(concretePath, Status::NeedsTimedInteraction) != CHIP_NO_ERROR ? Status::Failure : Status::Success;`
	`301`	`+ return AddStatus(concretePath, Status::NeedsTimedInteraction) != CHIP_NO_ERROR ? Status::Failure : Status::Success;`
`302`	`302`	`}`
`303`	`303`
`304`	`304`	`if (CommandIsFabricScoped(concretePath.mClusterId, concretePath.mCommandId))`
`@@ -312,7 +312,7 @@ Status CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommandElem`
`312`	`312`	`{`
`313`	`313`	`// TODO: when wildcard invokes are supported, discard a`
`314`	`314`	`// wildcard-expanded path instead of returning a status.`
`315`		`- return FallibleAddStatus(concretePath, Status::UnsupportedAccess) != CHIP_NO_ERROR ? Status::Failure : Status::Success;`
	`315`	`+ return AddStatus(concretePath, Status::UnsupportedAccess) != CHIP_NO_ERROR ? Status::Failure : Status::Success;`
`316`	`316`	`}`
`317`	`317`	`}`
`318`	`318`
`@@ -337,7 +337,7 @@ Status CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommandElem`
`337`	`337`	`exit:`
`338`	`338`	`if (err != CHIP_NO_ERROR)`
`339`	`339`	`{`
`340`		`- return FallibleAddStatus(concretePath, Status::InvalidCommand) != CHIP_NO_ERROR ? Status::Failure : Status::Success;`
	`340`	`+ return AddStatus(concretePath, Status::InvalidCommand) != CHIP_NO_ERROR ? Status::Failure : Status::Success;`
`341`	`341`	`}`
`342`	`342`
`343`	`343`	`// We have handled the error status above and put the error status in response, now return success status so we can process`
`@@ -468,31 +468,31 @@ CHIP_ERROR CommandHandler::AddStatusInternal(const ConcreteCommandPath & aComman`
`468`	`468`	`return FinishStatus();`
`469`	`469`	`}`
`470`	`470`
`471`		`-void CommandHandler::AddStatus(const ConcreteCommandPath & aCommandPath, const Protocols::InteractionModel::Status aStatus,`
`472`		`- const char * context)`
	`471`	`+CHIP_ERROR CommandHandler::AddStatus(const ConcreteCommandPath & aCommandPath, const Status aStatus)`
`473`	`472`	`{`
`474`		`-`
`475`		`- VerifyOrDie(FallibleAddStatus(aCommandPath, aStatus, context) == CHIP_NO_ERROR);`
	`473`	`+ return AddStatusInternal(aCommandPath, StatusIB(aStatus));`
`476`	`474`	`}`
`477`	`475`
`478`		`-CHIP_ERROR CommandHandler::FallibleAddStatus(const ConcreteCommandPath & path, const Protocols::InteractionModel::Status status,`
`479`		`- const char * context)`
	`476`	`+void CommandHandler::AddStatusAndLogIfFailure(const ConcreteCommandPath & aCommandPath, const Status aStatus, const char * aMessage)`
`480`	`477`	`{`
`481`		`-`
`482`		`- if (status != Status::Success)`
	`478`	`+ if (aStatus != Status::Success)`
`483`	`479`	`{`
`484`		`- if (context == nullptr)`
`485`		`- {`
`486`		`- context = "no additional context";`
`487`		`- }`
`488`		`-`
`489`	`480`	`ChipLogError(DataManagement,`
`490`		`- "Endpoint=%u Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI " status " ChipLogFormatIMStatus " (%s)",`
`491`		`- path.mEndpointId, ChipLogValueMEI(path.mClusterId), ChipLogValueMEI(path.mCommandId),`
`492`		`- ChipLogValueIMStatus(status), context);`
	`481`	`+ "Failed to handle on Endpoint=%u Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI`
	`482`	`+ " with " ChipLogFormatIMStatus ": %s",`
	`483`	`+ aCommandPath.mEndpointId, ChipLogValueMEI(aCommandPath.mClusterId), ChipLogValueMEI(aCommandPath.mCommandId),`
	`484`	`+ ChipLogValueIMStatus(aStatus), aMessage);`
`493`	`485`	`}`
`494`	`486`
`495`		`- return AddStatusInternal(path, StatusIB(status));`
	`487`	`+ CHIP_ERROR err = AddStatus(aCommandPath, aStatus);`
	`488`	`+ if (err != CHIP_NO_ERROR)`
	`489`	`+ {`
	`490`	`+ ChipLogError(DataManagement,`
	`491`	`+ "Failed to set status on Endpoint=%u Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI`
	`492`	`+ ": %" CHIP_ERROR_FORMAT,`
	`493`	`+ aCommandPath.mEndpointId, ChipLogValueMEI(aCommandPath.mClusterId), ChipLogValueMEI(aCommandPath.mCommandId),`
	`494`	`+ err.Format());`
	`495`	`+ }`
`496`	`496`	`}`
`497`	`497`
`498`	`498`	`CHIP_ERROR CommandHandler::AddClusterSpecificSuccess(const ConcreteCommandPath & aCommandPath, ClusterStatus aClusterStatus)`
Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ class CommandResponseHelper`
`53`	`53`
`54`	`54`	`CHIP_ERROR Failure(Protocols::InteractionModel::Status aStatus)`
`55`	`55`	`{`
`56`		`- CHIP_ERROR err = mCommandHandler->FallibleAddStatus(mCommandPath, aStatus);`
	`56`	`+ CHIP_ERROR err = mCommandHandler->AddStatus(mCommandPath, aStatus);`
`57`	`57`	`if (err == CHIP_NO_ERROR)`
`58`	`58`	`{`
`59`	`59`	`mSentResponse = true;`
Original file line number	Diff line number	Diff line change
`@@ -286,7 +286,10 @@ void emberAfBarrierControlClusterServerTickCallback(EndpointId endpoint)`
`286`	`286`
`287`	`287`	`static void sendDefaultResponse(app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath, Status status)`
`288`	`288`	`{`
`289`		`- commandObj->AddStatus(commandPath, status);`
	`289`	`+ if (commandObj->AddStatus(commandPath, status) != CHIP_NO_ERROR)`
	`290`	`+ {`
	`291`	`+ ChipLogProgress(Zcl, "Failed to send default response");`
	`292`	`+ }`
`290`	`293`	`}`
`291`	`294`
`292`	`295`	`bool emberAfBarrierControlClusterBarrierControlGoToPercentCallback(`
Original file line number	Diff line number	Diff line change
`@@ -3314,20 +3314,23 @@ bool DoorLockServer::RemoteOperationEnabled(chip::EndpointId endpointId) const`
`3314`	`3314`	`mode != OperatingModeEnum::kPrivacy && mode != OperatingModeEnum::kNoRemoteLockUnlock;`
`3315`	`3315`	`}`
`3316`	`3316`
`3317`		`-void DoorLockServer::sendClusterResponse(chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,`
`3318`		`- EmberAfStatus status)`
	`3317`	`+CHIP_ERROR DoorLockServer::sendClusterResponse(chip::app::CommandHandler * commandObj,`
	`3318`	`+ const chip::app::ConcreteCommandPath & commandPath, EmberAfStatus status)`
`3319`	`3319`	`{`
`3320`	`3320`	`VerifyOrDie(nullptr != commandObj);`
`3321`	`3321`
	`3322`	`+ auto err = CHIP_NO_ERROR;`
`3322`	`3323`	`auto statusAsInteger = to_underlying(status);`
`3323`	`3324`	`if (statusAsInteger == to_underlying(DlStatus::kOccupied) \|\| statusAsInteger == to_underlying(DlStatus::kDuplicate))`
`3324`	`3325`	`{`
`3325`		`- VerifyOrDie(commandObj->AddClusterSpecificFailure(commandPath, static_cast<chip::ClusterStatus>(status)) == CHIP_NO_ERROR);`
	`3326`	`+ err = commandObj->AddClusterSpecificFailure(commandPath, static_cast<chip::ClusterStatus>(status));`
`3326`	`3327`	`}`
`3327`	`3328`	`else`
`3328`	`3329`	`{`
`3329`		`- commandObj->AddStatus(commandPath, ToInteractionModelStatus(status));`
	`3330`	`+ err = commandObj->AddStatus(commandPath, ToInteractionModelStatus(status));`
`3330`	`3331`	`}`
	`3332`	`+`
	`3333`	`+ return err;`
`3331`	`3334`	`}`
`3332`	`3335`
`3333`	`3336`	`EmberAfDoorLockEndpointContext * DoorLockServer::getContext(chip::EndpointId endpointId)`
Original file line number	Diff line number	Diff line change
`@@ -414,13 +414,13 @@ bool GetProviderAndFabric(chip::app::CommandHandler * commandObj, const chip::ap`
`414`	`414`
`415`	`415`	`if (nullptr == provider)`
`416`	`416`	`{`
`417`		`- commandObj->AddStatus(commandPath, Status::Failure, "Internal consistency error on provider!");`
	`417`	`+ commandObj->AddStatusAndLogIfFailure(commandPath, Status::Failure, "Internal consistency error on provider!");`
`418`	`418`	`return false;`
`419`	`419`	`}`
`420`	`420`
`421`	`421`	`if (nullptr == fabric)`
`422`	`422`	`{`
`423`		`- commandObj->AddStatus(commandPath, Status::Failure, "Internal consistency error on access fabric!");`
	`423`	`+ commandObj->AddStatusAndLogIfFailure(commandPath, Status::Failure, "Internal consistency error on access fabric!");`
`424`	`424`	`return false;`
`425`	`425`	`}`
`426`	`426`
`@@ -460,7 +460,7 @@ bool emberAfGroupKeyManagementClusterKeySetWriteCallback(`
`460`	`460`	`Status status = ValidateKeySetWriteArguments(commandData);`
`461`	`461`	`if (status != Status::Success)`
`462`	`462`	`{`
`463`		`- commandObj->AddStatus(commandPath, status, "Failure to validate KeySet data dependencies.");`
	`463`	`+ commandObj->AddStatusAndLogIfFailure(commandPath, status, "Failure to validate KeySet data dependencies.");`
`464`	`464`	`return true;`
`465`	`465`	`}`
`466`	`466`
`@@ -470,7 +470,8 @@ bool emberAfGroupKeyManagementClusterKeySetWriteCallback(`
`470`	`470`	`// supported by the server, because it is ... a new value unrecognized`
`471`	`471`	`// by a legacy server, then the server SHALL generate a general`
`472`	`472`	`// constraint error`
`473`		`- commandObj->AddStatus(commandPath, Status::ConstraintError, "Received unknown GroupKeySecurityPolicyEnum value");`
	`473`	`+ commandObj->AddStatusAndLogIfFailure(commandPath, Status::ConstraintError,`
	`474`	`+ "Received unknown GroupKeySecurityPolicyEnum value");`
`474`	`475`	`return true;`
`475`	`476`	`}`
`476`	`477`
`@@ -481,8 +482,8 @@ bool emberAfGroupKeyManagementClusterKeySetWriteCallback(`
`481`	`482`	`// any action attempting to set CacheAndSync in the`
`482`	`483`	`// GroupKeySecurityPolicy field SHALL fail with an INVALID_COMMAND`
`483`	`484`	`// error.`
`484`		`- commandObj->AddStatus(commandPath, Status::InvalidCommand,`
`485`		`- "Received a CacheAndSync GroupKeySecurityPolicyEnum when MCSP not supported");`
	`485`	`+ commandObj->AddStatusAndLogIfFailure(commandPath, Status::InvalidCommand,`
	`486`	`+ "Received a CacheAndSync GroupKeySecurityPolicyEnum when MCSP not supported");`
`486`	`487`	`return true;`
`487`	`488`	`}`
`488`	`489`
`@@ -528,7 +529,7 @@ bool emberAfGroupKeyManagementClusterKeySetWriteCallback(`
`528`	`529`	`err = provider->SetKeySet(fabric->GetFabricIndex(), compressed_fabric_id, keyset);`
`529`	`530`	`if (CHIP_ERROR_INVALID_LIST_LENGTH == err)`
`530`	`531`	`{`
`531`		`- commandObj->AddStatus(commandPath, Status::ResourceExhausted, "Not enough space left to add a new KeySet");`
	`532`	`+ commandObj->AddStatusAndLogIfFailure(commandPath, Status::ResourceExhausted, "Not enough space left to add a new KeySet");`
`532`	`533`	`return true;`
`533`	`534`	`}`
`534`	`535`
`@@ -564,7 +565,7 @@ bool emberAfGroupKeyManagementClusterKeySetReadCallback(`
`564`	`565`	`if (CHIP_NO_ERROR != provider->GetKeySet(fabricIndex, commandData.groupKeySetID, keyset))`
`565`	`566`	`{`
`566`	`567`	`// KeySet ID not found`
`567`		`- commandObj->AddStatus(commandPath, Status::NotFound, "Keyset ID not found in KeySetRead");`
	`568`	`+ commandObj->AddStatusAndLogIfFailure(commandPath, Status::NotFound, "Keyset ID not found in KeySetRead");`
`568`	`569`	`return true;`
`569`	`570`	`}`
`570`	`571`
`@@ -628,7 +629,8 @@ bool emberAfGroupKeyManagementClusterKeySetRemoveCallback(`
`628`	`629`	`{`
`629`	`630`	`// SPEC: This command SHALL fail with an INVALID_COMMAND status code back to the initiator if the GroupKeySetID being`
`630`	`631`	`// removed is 0, which is the Key Set associated with the Identity Protection Key (IPK).`
`631`		`- commandObj->AddStatus(commandPath, Status::InvalidCommand, "Attempted to KeySetRemove the identity protection key!");`
	`632`	`+ commandObj->AddStatusAndLogIfFailure(commandPath, Status::InvalidCommand,`
	`633`	`+ "Attempted to KeySetRemove the identity protection key!");`
`632`	`634`	`return true;`
`633`	`635`	`}`
`634`	`636`
`@@ -647,7 +649,7 @@ bool emberAfGroupKeyManagementClusterKeySetRemoveCallback(`
`647`	`649`	`}`
`648`	`650`
`649`	`651`	`// Send status response.`
`650`		`- commandObj->AddStatus(commandPath, status, "KeySetRemove failed");`
	`652`	`+ commandObj->AddStatusAndLogIfFailure(commandPath, status, "KeySetRemove failed");`
`651`	`653`	`return true;`
`652`	`654`	`}`
`653`	`655`
`@@ -699,7 +701,7 @@ bool emberAfGroupKeyManagementClusterKeySetReadAllIndicesCallback(`
`699`	`701`	`auto keysIt = provider->IterateKeySets(fabricIndex);`
`700`	`702`	`if (nullptr == keysIt)`
`701`	`703`	`{`
`702`		`- commandObj->AddStatus(commandPath, Status::Failure, "Failed iteration of key set indices!");`
	`704`	`+ commandObj->AddStatusAndLogIfFailure(commandPath, Status::Failure, "Failed iteration of key set indices!");`
`703`	`705`	`return true;`
`704`	`706`	`}`
`705`	`707`
Original file line number	Diff line number	Diff line change
`@@ -351,7 +351,11 @@ bool emberAfGroupsClusterAddGroupIfIdentifyingCallback(app::CommandHandler * com`
`351`	`351`	`status = GroupAdd(fabricIndex, endpointId, groupId, groupName);`
`352`	`352`	`}`
`353`	`353`
`354`		`- commandObj->AddStatus(commandPath, status);`
	`354`	`+ CHIP_ERROR sendErr = commandObj->AddStatus(commandPath, status);`
	`355`	`+ if (CHIP_NO_ERROR != sendErr)`
	`356`	`+ {`
	`357`	`+ ChipLogDetail(Zcl, "Groups: failed to send %s: %" CHIP_ERROR_FORMAT, "status_response", sendErr.Format());`
	`358`	`+ }`
`355`	`359`	`return true;`
`356`	`360`	`}`
`357`	`361`
Original file line number	Diff line number	Diff line change
`@@ -771,8 +771,7 @@ bool emberAfWindowCoveringClusterStopMotionCallback(app::CommandHandler * comman`
`771`	`771`	`}`
`772`	`772`	`}`
`773`	`773`
`774`		`- commandObj->AddStatus(commandPath, Status::Success);`
`775`		`- return true;`
	`774`	`+ return CHIP_NO_ERROR == commandObj->AddStatus(commandPath, Status::Success);`
`776`	`775`	`}`
`777`	`776`
`778`	`777`	`/**`