From 5ba08b1d265b41717a697bb9534acb2ea9699db9 Mon Sep 17 00:00:00 2001
From: cyan317 <120398799+cindyyan317@users.noreply.github.com>
Date: Mon, 17 Jun 2024 11:52:39 +0100
Subject: [PATCH] Improve etl check (#1465)

Fix #1444
---
 src/etl/LoadBalancer.cpp             |  9 ++++----
 tests/unit/etl/LoadBalancerTests.cpp | 33 ++++++++++++++++++----------
 2 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/src/etl/LoadBalancer.cpp b/src/etl/LoadBalancer.cpp
index bd4000ce3..f42a3d1d4 100644
--- a/src/etl/LoadBalancer.cpp
+++ b/src/etl/LoadBalancer.cpp
@@ -123,10 +123,8 @@ LoadBalancer::LoadBalancer(
         auto const stateOpt = ETLState::fetchETLStateFromSource(*source);
 
         if (!stateOpt) {
-            checkOnETLFailure(fmt::format(
-                "Failed to fetch ETL state from source = {} Please check the configuration and network",
-                source->toString()
-            ));
+            LOG(log_.warn()) << "Failed to fetch ETL state from source = " << source->toString()
+                             << " Please check the configuration and network";
         } else if (etlState_ && etlState_->networkID && stateOpt->networkID &&
                    etlState_->networkID != stateOpt->networkID) {
             checkOnETLFailure(fmt::format(
@@ -142,6 +140,9 @@ LoadBalancer::LoadBalancer(
         LOG(log_.info()) << "Added etl source - " << sources_.back()->toString();
     }
 
+    if (!etlState_)
+        checkOnETLFailure("Failed to fetch ETL state from any source. Please check the configuration and network");
+
     if (sources_.empty())
         checkOnETLFailure("No ETL sources configured. Please check the configuration");
 
diff --git a/tests/unit/etl/LoadBalancerTests.cpp b/tests/unit/etl/LoadBalancerTests.cpp
index 4bf9e1778..50ad004db 100644
--- a/tests/unit/etl/LoadBalancerTests.cpp
+++ b/tests/unit/etl/LoadBalancerTests.cpp
@@ -107,30 +107,42 @@ TEST_F(LoadBalancerConstructorTests, forwardingTimeoutPassedToSourceFactory)
     makeLoadBalancer();
 }
 
-TEST_F(LoadBalancerConstructorTests, fetchETLState_Source0Fails)
+TEST_F(LoadBalancerConstructorTests, fetchETLState_AllSourcesFail)
 {
-    EXPECT_CALL(sourceFactory_, makeSource).Times(1);
+    EXPECT_CALL(sourceFactory_, makeSource).Times(2);
     EXPECT_CALL(sourceFactory_.sourceAt(0), forwardToRippled).WillOnce(Return(std::nullopt));
-    EXPECT_CALL(sourceFactory_.sourceAt(0), toString);
+    EXPECT_CALL(sourceFactory_.sourceAt(1), forwardToRippled).WillOnce(Return(std::nullopt));
     EXPECT_THROW({ makeLoadBalancer(); }, std::logic_error);
 }
 
-TEST_F(LoadBalancerConstructorTests, fetchETLState_Source0ReturnsError)
+TEST_F(LoadBalancerConstructorTests, fetchETLState_AllSourcesReturnError)
 {
-    EXPECT_CALL(sourceFactory_, makeSource).Times(1);
+    EXPECT_CALL(sourceFactory_, makeSource).Times(2);
     EXPECT_CALL(sourceFactory_.sourceAt(0), forwardToRippled)
         .WillOnce(Return(boost::json::object{{"error", "some error"}}));
-    EXPECT_CALL(sourceFactory_.sourceAt(0), toString);
+    EXPECT_CALL(sourceFactory_.sourceAt(1), forwardToRippled)
+        .WillOnce(Return(boost::json::object{{"error", "some error"}}));
     EXPECT_THROW({ makeLoadBalancer(); }, std::logic_error);
 }
 
-TEST_F(LoadBalancerConstructorTests, fetchETLState_Source1Fails)
+TEST_F(LoadBalancerConstructorTests, fetchETLState_Source1Fails0OK)
 {
     EXPECT_CALL(sourceFactory_, makeSource).Times(2);
     EXPECT_CALL(sourceFactory_.sourceAt(0), forwardToRippled).WillOnce(Return(boost::json::object{}));
     EXPECT_CALL(sourceFactory_.sourceAt(1), forwardToRippled).WillOnce(Return(std::nullopt));
-    EXPECT_CALL(sourceFactory_.sourceAt(1), toString);
-    EXPECT_THROW({ makeLoadBalancer(); }, std::logic_error);
+    EXPECT_CALL(sourceFactory_.sourceAt(0), run);
+    EXPECT_CALL(sourceFactory_.sourceAt(1), run);
+    makeLoadBalancer();
+}
+
+TEST_F(LoadBalancerConstructorTests, fetchETLState_Source0Fails1OK)
+{
+    EXPECT_CALL(sourceFactory_, makeSource).Times(2);
+    EXPECT_CALL(sourceFactory_.sourceAt(0), forwardToRippled).WillOnce(Return(std::nullopt));
+    EXPECT_CALL(sourceFactory_.sourceAt(1), forwardToRippled).WillOnce(Return(boost::json::object{}));
+    EXPECT_CALL(sourceFactory_.sourceAt(0), run);
+    EXPECT_CALL(sourceFactory_.sourceAt(1), run);
+    makeLoadBalancer();
 }
 
 TEST_F(LoadBalancerConstructorTests, fetchETLState_DifferentNetworkID)
@@ -144,13 +156,12 @@ TEST_F(LoadBalancerConstructorTests, fetchETLState_DifferentNetworkID)
     EXPECT_THROW({ makeLoadBalancer(); }, std::logic_error);
 }
 
-TEST_F(LoadBalancerConstructorTests, fetchETLState_Source1FailsButAllowNoEtlIsTrue)
+TEST_F(LoadBalancerConstructorTests, fetchETLState_AllSourcesFailButAllowNoEtlIsTrue)
 {
     EXPECT_CALL(sourceFactory_, makeSource).Times(2);
     EXPECT_CALL(sourceFactory_.sourceAt(0), forwardToRippled).WillOnce(Return(boost::json::object{}));
     EXPECT_CALL(sourceFactory_.sourceAt(0), run);
     EXPECT_CALL(sourceFactory_.sourceAt(1), forwardToRippled).WillOnce(Return(std::nullopt));
-    EXPECT_CALL(sourceFactory_.sourceAt(1), toString);
     EXPECT_CALL(sourceFactory_.sourceAt(1), run);
 
     configJson_.as_object()["allow_no_etl"] = true;