Improve the Darwin logging when we cancel dns-sd browse/resolve operations. (#26072)

Right now we are treating those as "timeout" and logging that, which really
confuses people reading the logs.  We should log them as cancellations instead.

There's no obvious kDNSServiceErr_* value for "cancelled", so allow doing
GenericContext::Finalize with a CHIP_ERROR.

Author: bzbarsky-apple

src/platform/Darwin/DnssdContexts.cpp

@@ -112,25 +112,35 @@ DNSServiceProtocol GetProtocol(const chip::Inet::IPAddressType & addressType)
 namespace chip {
 namespace Dnssd {
 
-CHIP_ERROR GenericContext::Finalize(DNSServiceErrorType err)
+CHIP_ERROR GenericContext::FinalizeInternal(const char * errorStr, CHIP_ERROR err)
 {
     if (MdnsContexts::GetInstance().Has(this) == CHIP_NO_ERROR)
     {
-        if (kDNSServiceErr_NoError == err)
+        if (CHIP_NO_ERROR == err)
         {
             DispatchSuccess();
         }
         else
         {
-            DispatchFailure(err);
+            DispatchFailure(errorStr, err);
         }
     }
     else
     {
         chip::Platform::Delete(this);
     }
 
-    return Error::ToChipError(err);
+    return err;
+}
+
+CHIP_ERROR GenericContext::Finalize(CHIP_ERROR err)
+{
+    return FinalizeInternal(err.AsString(), err);
+}
+
+CHIP_ERROR GenericContext::Finalize(DNSServiceErrorType err)
+{
+    return FinalizeInternal(Error::ToString(err), Error::ToChipError(err));
 }
 
 MdnsContexts::~MdnsContexts()
@@ -289,10 +299,10 @@ RegisterContext::RegisterContext(const char * sType, const char * instanceName,
     mInstanceName = instanceName;
 }
 
-void RegisterContext::DispatchFailure(DNSServiceErrorType err)
+void RegisterContext::DispatchFailure(const char * errorStr, CHIP_ERROR err)
 {
-    ChipLogError(Discovery, "Mdns: Register failure (%s)", Error::ToString(err));
-    callback(context, nullptr, nullptr, Error::ToChipError(err));
+    ChipLogError(Discovery, "Mdns: Register failure (%s)", errorStr);
+    callback(context, nullptr, nullptr, err);
     MdnsContexts::GetInstance().Remove(this);
 }
 
@@ -316,10 +326,10 @@ BrowseContext::BrowseContext(void * cbContext, DnssdBrowseCallback cb, DnssdServ
     protocol = cbContextProtocol;
 }
 
-void BrowseContext::DispatchFailure(DNSServiceErrorType err)
+void BrowseContext::DispatchFailure(const char * errorStr, CHIP_ERROR err)
 {
-    ChipLogError(Discovery, "Mdns: Browse failure (%s)", Error::ToString(err));
-    callback(context, nullptr, 0, true, Error::ToChipError(err));
+    ChipLogError(Discovery, "Mdns: Browse failure (%s)", errorStr);
+    callback(context, nullptr, 0, true, err);
     MdnsContexts::GetInstance().Remove(this);
 }
 
@@ -353,14 +363,14 @@ ResolveContext::ResolveContext(void * cbContext, DnssdResolveCallback cb, chip::
 
 ResolveContext::~ResolveContext() {}
 
-void ResolveContext::DispatchFailure(DNSServiceErrorType err)
+void ResolveContext::DispatchFailure(const char * errorStr, CHIP_ERROR err)
 {
-    ChipLogError(Discovery, "Mdns: Resolve failure (%s)", Error::ToString(err));
+    ChipLogError(Discovery, "Mdns: Resolve failure (%s)", errorStr);
     // Remove before dispatching, so calls back into
     // ChipDnssdResolveNoLongerNeeded don't find us and try to also remove us.
     bool needDelete = MdnsContexts::GetInstance().RemoveWithoutDeleting(this);
 
-    callback(context, nullptr, Span<Inet::IPAddress>(), Error::ToChipError(err));
+    callback(context, nullptr, Span<Inet::IPAddress>(), err);
 
     if (needDelete)
     {

src/platform/Darwin/DnssdImpl.cpp

@@ -465,11 +465,11 @@ CHIP_ERROR ChipDnssdStopBrowse(intptr_t browseIdentifier)
         return CHIP_ERROR_NOT_FOUND;
     }
 
-    // Just treat this as a timeout error.  Don't bother delivering the partial
+    // We have been canceled.  Don't bother delivering the partial
     // results we have queued up in the BrowseContext, if any.  In practice
     // there shouldn't be anything there long-term anyway.
     //
-    // Make sure to time out all the resolves first, before we time out the
+    // Make sure to cancel all the resolves first, before we cancel the
     // browse (just to avoid dangling pointers in the resolves, even though we
     // only use them for equality compares).
     std::vector<GenericContext *> resolves;
@@ -481,10 +481,10 @@ CHIP_ERROR ChipDnssdStopBrowse(intptr_t browseIdentifier)
 
     for (auto & resolve : resolves)
     {
-        resolve->Finalize(kDNSServiceErr_Timeout);
+        resolve->Finalize(CHIP_ERROR_CANCELLED);
     }
 
-    ctx->Finalize(kDNSServiceErr_Timeout);
+    ctx->Finalize(CHIP_ERROR_CANCELLED);
     return CHIP_NO_ERROR;
 }
 
@@ -511,11 +511,11 @@ void ChipDnssdResolveNoLongerNeeded(const char * instanceName)
     if (*existingCtx->consumerCounter == 0)
     {
         // No more consumers; clear out all of these resolves so they don't
-        // stick around.  Dispatch timeout failure on all of them to make sure
-        // whatever kicked them off cleans up resources as needed.
+        // stick around.  Dispatch a "cancelled" failure on all of them to make
+        // sure whatever kicked them off cleans up resources as needed.
         do
         {
-            existingCtx->Finalize(kDNSServiceErr_Timeout);
+            existingCtx->Finalize(CHIP_ERROR_CANCELLED);
             existingCtx = MdnsContexts::GetInstance().GetExistingResolveForInstanceName(instanceName);
         } while (existingCtx != nullptr);
     }

src/platform/Darwin/DnssdImpl.h

@@ -44,9 +44,14 @@ struct GenericContext
 
     virtual ~GenericContext() {}
 
+    CHIP_ERROR Finalize(CHIP_ERROR err);
     CHIP_ERROR Finalize(DNSServiceErrorType err = kDNSServiceErr_NoError);
-    virtual void DispatchFailure(DNSServiceErrorType err) = 0;
-    virtual void DispatchSuccess()                        = 0;
+
+    virtual void DispatchFailure(const char * errorStr, CHIP_ERROR err) = 0;
+    virtual void DispatchSuccess()                                      = 0;
+
+private:
+    CHIP_ERROR FinalizeInternal(const char * errorStr, CHIP_ERROR err);
 };
 
 struct RegisterContext;
@@ -130,7 +135,7 @@ struct RegisterContext : public GenericContext
     RegisterContext(const char * sType, const char * instanceName, DnssdPublishCallback cb, void * cbContext);
     virtual ~RegisterContext() { mHostNameRegistrar.Unregister(); }
 
-    void DispatchFailure(DNSServiceErrorType err) override;
+    void DispatchFailure(const char * errorStr, CHIP_ERROR err) override;
     void DispatchSuccess() override;
 
     bool matches(const char * sType) { return mType.compare(sType) == 0; }
@@ -145,7 +150,7 @@ struct BrowseContext : public GenericContext
     BrowseContext(void * cbContext, DnssdBrowseCallback cb, DnssdServiceProtocol cbContextProtocol);
     virtual ~BrowseContext() {}
 
-    void DispatchFailure(DNSServiceErrorType err) override;
+    void DispatchFailure(const char * errorStr, CHIP_ERROR err) override;
     void DispatchSuccess() override;
 
     // Dispatch what we have found so far, but don't stop browsing.
@@ -195,7 +200,7 @@ struct ResolveContext : public GenericContext
                    std::shared_ptr<uint32_t> && consumerCounterToUse);
     virtual ~ResolveContext();
 
-    void DispatchFailure(DNSServiceErrorType err) override;
+    void DispatchFailure(const char * errorStr, CHIP_ERROR err) override;
     void DispatchSuccess() override;
 
     CHIP_ERROR OnNewAddress(uint32_t interfaceId, const struct sockaddr * address);