Fix handling of errors in Darwin framework OTA delegate bridge. (#22437)

This allows the framework consumer to return an error that will be sent as an error status to the OTA provider client.

Fixes project-chip/connectedhomeip#20653

Author: bzbarsky-apple

examples/darwin-framework-tool/commands/provider/OTAProviderDelegate.mm

@@ -46,28 +46,19 @@ - (void)handleQueryImageForNodeID:(NSNumber * _Nonnull)nodeID
                 completionHandler:(void (^_Nonnull)(MTROtaSoftwareUpdateProviderClusterQueryImageResponseParams * _Nullable data,
                                       NSError * _Nullable error))completionHandler
 {
-    NSError * error;
-
     auto isBDXProtocolSupported =
         [params.protocolsSupported containsObject:@(MTROtaSoftwareUpdateProviderOTADownloadProtocolBDXSynchronous)];
     if (!isBDXProtocolSupported) {
         _selectedCandidate.status = @(MTROtaSoftwareUpdateProviderOTAQueryStatusDownloadProtocolNotSupported);
-        error =
-            [[NSError alloc] initWithDomain:@"OTAProviderDomain"
-                                       code:MTRErrorCodeGeneralError
-                                   userInfo:@{ NSLocalizedDescriptionKey : NSLocalizedString(@"Protocol is not supported.", nil) }];
-        completionHandler(_selectedCandidate, error);
+        completionHandler(_selectedCandidate, nil);
         return;
     }
 
     auto hasCandidate = [self SelectOTACandidate:params.vendorId rPID:params.productId rSV:params.softwareVersion];
     if (!hasCandidate) {
         NSLog(@"Unable to select OTA Image.");
         _selectedCandidate.status = @(MTROtaSoftwareUpdateProviderOTAQueryStatusNotAvailable);
-        error = [[NSError alloc]
-            initWithDomain:@"OTAProviderDomain"
-                      code:MTRErrorCodeInvalidState
-                  userInfo:@{ NSLocalizedDescriptionKey : NSLocalizedString(@"Unable to select Candidate.", nil) }];
+        completionHandler(_selectedCandidate, nil);
         return;
     }
 
@@ -78,7 +69,7 @@ - (void)handleQueryImageForNodeID:(NSNumber * _Nonnull)nodeID
         _selectedCandidate.userConsentNeeded
             = (_userConsentState == OTAProviderUserUnknown || _userConsentState == OTAProviderUserDenied) ? @(1) : @(0);
         NSLog(@"User Consent Needed: %@", _selectedCandidate.userConsentNeeded);
-        completionHandler(_selectedCandidate, error);
+        completionHandler(_selectedCandidate, nil);
         return;
     }
 
@@ -101,7 +92,7 @@ - (void)handleQueryImageForNodeID:(NSNumber * _Nonnull)nodeID
         break;
     }
     _selectedCandidate.status = @(_queryImageStatus);
-    completionHandler(_selectedCandidate, error);
+    completionHandler(_selectedCandidate, nil);
 }
 
 - (void)handleApplyUpdateRequestForNodeID:(NSNumber * _Nonnull)nodeID

src/darwin/Framework/CHIP/MTROTAProviderDelegate.h

@@ -31,6 +31,10 @@ NS_ASSUME_NONNULL_BEGIN
  * Notify the delegate when the query image command is received from some node.
  * The controller identifies the fabric the node is on, and the nodeID
  * identifies the node within that fabric.
+ *
+ * If completionHandler is passed a non-nil error, that will be converted into
+ * an error response to the client.  Otherwise it must have a non-nil data,
+ * which will be returned to the client.
  */
 - (void)handleQueryImageForNodeID:(NSNumber *)nodeID
                        controller:(MTRDeviceController *)controller
@@ -42,6 +46,10 @@ NS_ASSUME_NONNULL_BEGIN
  * Notify the delegate when the apply update request command is received from
  * some node.  The controller identifies the fabric the node is on, and the
  * nodeID identifies the node within that fabric.
+ *
+ * If completionHandler is passed a non-nil error, that will be converted into
+ * an error response to the client.  Otherwise it must have a non-nil data,
+ * which will be returned to the client.
  */
 - (void)handleApplyUpdateRequestForNodeID:(NSNumber *)nodeID
                                controller:(MTRDeviceController *)controller
@@ -53,6 +61,9 @@ NS_ASSUME_NONNULL_BEGIN
  * Notify the delegate when the notify update applied command is received from
  * some node.  The controller identifies the fabric the node is on, and the
  * nodeID identifies the node within that fabric.
+ *
+ * If completionHandler is passed a non-nil error, that will be converted into
+ * an error response to the client.  Otherwise a success response will be sent.
  */
 - (void)handleNotifyUpdateAppliedForNodeID:(NSNumber *)nodeID
                                 controller:(MTRDeviceController *)controller

src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm

@@ -378,6 +378,60 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
     *outNodeId = desc.subject;
     return true;
 }
+
+// Ensures we have a usable CommandHandler and do not have an error.
+//
+// When this function returns non-null, it's safe to go ahead and use the return
+// value to send a response.
+//
+// When this function returns null, the CommandHandler::Handle should not be
+// used anymore.
+CommandHandler * _Nullable EnsureValidState(
+    CommandHandler::Handle & handle, const ConcreteCommandPath & cachedCommandPath, const char * prefix, NSError * _Nullable error)
+{
+    CommandHandler * handler = handle.Get();
+    if (handler == nullptr) {
+        ChipLogError(Controller, "%s: no CommandHandler to send response", prefix);
+        return nullptr;
+    }
+
+    if (error != nil) {
+        auto * desc = [error description];
+        auto err = [MTRError errorToCHIPErrorCode:error];
+        ChipLogError(Controller, "%s: application returned error: '%s', sending error: '%s'", prefix,
+            [desc cStringUsingEncoding:NSUTF8StringEncoding], chip::ErrorStr(err));
+
+        handler->AddStatus(cachedCommandPath, StatusIB(err).mStatus);
+        handle.Release();
+        return nullptr;
+    }
+
+    return handler;
+}
+
+// Ensures we have a usable CommandHandler and that our args don't involve any
+// errors, for the case when we have data to send back.
+//
+// When this function returns non-null, it's safe to go ahead and use whatever
+// object "data" points to to add a response to the command.
+//
+// When this function returns null, the CommandHandler::Handle should not be
+// used anymore.
+CommandHandler * _Nullable EnsureValidState(CommandHandler::Handle & handle, const ConcreteCommandPath & cachedCommandPath,
+    const char * prefix, NSObject * _Nullable data, NSError * _Nullable error)
+{
+    CommandHandler * handler = EnsureValidState(handle, cachedCommandPath, prefix, error);
+    VerifyOrReturnValue(handler != nullptr, nullptr);
+
+    if (data == nil) {
+        ChipLogError(Controller, "%s: no data to send as a response", prefix);
+        handler->AddStatus(cachedCommandPath, Protocols::InteractionModel::Status::Failure);
+        handle.Release();
+        return nullptr;
+    }
+
+    return handler;
+}
 } // anonymous namespace
 
 void MTROTAProviderDelegateBridge::HandleQueryImage(
@@ -403,9 +457,12 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
     auto completionHandler = ^(
         MTROtaSoftwareUpdateProviderClusterQueryImageResponseParams * _Nullable data, NSError * _Nullable error) {
         dispatch_async(mWorkQueue, ^{
-            CommandHandler * handler = handle.Get();
+            CommandHandler * handler = EnsureValidState(handle, cachedCommandPath, "QueryImage", data, error);
             VerifyOrReturn(handler != nullptr);
 
+            ChipLogDetail(Controller, "QueryImage: application responded with: %s",
+                [[data description] cStringUsingEncoding:NSUTF8StringEncoding]);
+
             Commands::QueryImageResponse::Type response;
             ConvertFromQueryImageResponseParms(data, response);
 
@@ -472,9 +529,12 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
     auto completionHandler
         = ^(MTROtaSoftwareUpdateProviderClusterApplyUpdateResponseParams * _Nullable data, NSError * _Nullable error) {
               dispatch_async(mWorkQueue, ^{
-                  CommandHandler * handler = handle.Get();
+                  CommandHandler * handler = EnsureValidState(handle, cachedCommandPath, "ApplyUpdateRequest", data, error);
                   VerifyOrReturn(handler != nullptr);
 
+                  ChipLogDetail(Controller, "ApplyUpdateRequest: application responded with: %s",
+                      [[data description] cStringUsingEncoding:NSUTF8StringEncoding]);
+
                   Commands::ApplyUpdateResponse::Type response;
                   ConvertFromApplyUpdateRequestResponseParms(data, response);
                   handler->AddResponse(cachedCommandPath, response);
@@ -509,7 +569,7 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
 
     auto completionHandler = ^(NSError * _Nullable error) {
         dispatch_async(mWorkQueue, ^{
-            CommandHandler * handler = handle.Get();
+            CommandHandler * handler = EnsureValidState(handle, cachedCommandPath, "NotifyUpdateApplied", error);
             VerifyOrReturn(handler != nullptr);
 
             handler->AddStatus(cachedCommandPath, Protocols::InteractionModel::Status::Success);