xds: update trace log to only print non-nullptr exceptions (envoyproxy#44026)

Commit Message: xds: update trace log to only print non-nullptr
exceptions
Additional Description:
Upon an xDS-error, trace-logs output the exception details. However
there are a few cases where an exception is created with nullptr passed.
This PR introduces defensive coding in the cases where the exception
details are printed out to the trace logs.

Risk Level: low - only trace-logs are impacted
Testing: Added unit test to avoid regression in this case.
Docs Changes: N/A
Release Notes: N/A - not user facing (trace logs)
Platform Specific Features: N/A

---------

Signed-off-by: Adi Suissa-Peleg <adip@google.com>

Author: adisuissa

source/common/upstream/od_cds_api_impl.cc

@@ -1,6 +1,7 @@
 #include "source/common/upstream/od_cds_api_impl.h"
 
 #include "source/common/common/assert.h"
+#include "source/common/common/enum_to_int.h"
 #include "source/common/grpc/common.h"
 
 #include "absl/strings/str_join.h"
@@ -264,9 +265,12 @@ class XdstpOdCdsApiImpl::XdstpOdcdsSubscriptionsManager : public Singleton::Inst
     }
     void onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason reason,
                               const EnvoyException* e) override {
-      ASSERT(reason != Envoy::Config::ConfigUpdateFailureReason::ConnectionFailure);
-      ENVOY_LOG(trace, "ODCDS-manager: error while fetching a single resource {}: {}",
-                resource_name_, e->what());
+      // The passed exception object is null iff the error reason is
+      // Envoy::Config::ConfigUpdateFailureReason::ConnectionFailure or
+      // Envoy::Config::ConfigUpdateFailureReason::FetchTimedout.
+      ENVOY_LOG(
+          trace, "ODCDS-manager: error while fetching a single resource {}: (reason = {}), {}",
+          resource_name_, enumToInt(reason), e == nullptr ? "exception not provided" : e->what());
       // If the resource wasn't previously updated, this sends a notification that it was removed,
       // so if there are any resources waiting for this one, they can proceed.
       if (!resource_was_updated_) {

test/common/upstream/xdstp_od_cds_api_impl_test.cc

@@ -182,6 +182,23 @@ TEST_F(XdstpOdCdsApiImplTest, SubscriptionFailure) {
                                          &e);
 }
 
+// Tests that a config update failure with a null exception pointer is handled correctly.
+TEST_F(XdstpOdCdsApiImplTest, SubscriptionFailureNullException) {
+  InSequence s;
+
+  const std::string cluster_name = "fake_cluster";
+  expectSingletonSubscription(cluster_name);
+  odcds_->updateOnDemand(cluster_name);
+
+  ASSERT_NE(odcds_callbacks_, nullptr);
+
+  EXPECT_CALL(cm_, addOrUpdateCluster(_, _, _)).Times(0);
+  EXPECT_CALL(notifier_, notifyMissingCluster(cluster_name));
+
+  odcds_callbacks_->onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason::UpdateRejected,
+                                         nullptr);
+}
+
 // Tests that an existing cluster is updated.
 TEST_F(XdstpOdCdsApiImplTest, ClusterUpdate) {
   InSequence s;